Fix mpsc::SyncSender spinning behavior

[ibraheemdev]

Resolves #106668.

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[taiki-e]

AFAIK std::sync::mpmc does not use is_completed together with snooze, so this looks like the correct fix for the current implementation of std::sync::mpmc.

Resolves #106668.

I think beta backport is also needed to completely fix #106668 because it marked as regression-from-stable-to-beta.

[taiki-e]

The current documentation around Backoff seems to be inconsistent with how it is actually used in the mpmc implementation, so (if we keep the current behavior) I would suggest updating it.

rust/library/std/src/sync/mpmc/utils.rs

Lines 107 to 110 in f8276c9

	/// Backs off in a lock-free loop.
	///
	/// This method should be used when we need to retry an operation because another thread made
	/// progress.

rust/library/std/src/sync/mpmc/utils.rs

Line 123 in f8276c9

/// Backs off in a blocking loop.

[Amanieu]

I reviewed the current usage and think it is fine.

@bors r+

Separately, I think the retry loop when sending a message should be removed (like we did with the similar loop in recv). The reasoning is the same: we don't want to waste CPU cycles spinning when trying to write to a full channel. Instead we should just go ahead and block immediately. But this can be a separate PR.

[bors]

📌 Commit 8917e99 has been approved by Amanieu

It is now in the queue for this repository.

[ibraheemdev]

Separately, I think the retry loop when sending a message should be removed (like we did with the similar loop in recv). The reasoning is the same: we don't want to waste CPU cycles spinning when trying to write to a full channel. Instead we should just go ahead and block immediately. But this can be a separate PR.

I agree, the original reasoning was that it is similar to acquiring a mutex, but the difference is that bounded channels are often used with heavy backpressure, so a quick unpark is often not expected.