docs: Documented Send and Sync requirements for Mutex + MutexGuard #135684

ranger-ross · 2025-01-18T14:33:14Z

This an attempt to continue where #123225 left off.

I did some light clean up from the work done in that PR.
I also documented the !Send + Sync implementations for MutexGuard to the best of my knowledge.
Let me know if I got anything wrong 😄

fixes #122856

cc: @IoaNNUwU

r? @joboet

joboet

Thank you for picking this up, it would be great to have some documentation here.

joboet · 2025-01-21T12:46:04Z

library/std/src/sync/poison/mutex.rs

+/// to safely send `Mutex` to another thread. This ensures that the protected
+/// data can be accessed safely from multiple threads without causing data races
+/// or other unsafe behavior.


The part about multiple threads is relevant for Sync. If you rely on Send, you have exclusive access anyways.

library/std/src/sync/poison/mutex.rs

joboet · 2025-01-21T12:50:29Z

library/std/src/sync/poison/mutex.rs

+/// a new thread. Because ownership always stays on the original thread `Sync`
+/// is safe to implement for [`MutexGuard`].
+/// See the `!Send` implementation on [`MutexGuard`] for more details.


I don't think ownership is very relevant here. The important part is that you can get a &T from &MutexGuard<T>, so as long as T is Sync (and therefore &T: Send), MutexGuard<T> can also be Sync.

Thanks, I think this is a much better way of framing it.

ranger-ross · 2025-01-22T13:23:12Z

@joboet Thanks.
I updated this PR based on your feedback.

lmk what you think :)

ranger-ross · 2025-01-22T13:29:05Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

joboet

Sorry for the language-lawyering. I'm studying a humanities subject, so I can be a bit picky about this stuff... 😄

library/std/src/sync/poison/mutex.rs

joboet · 2025-01-24T13:36:27Z

library/std/src/sync/poison/mutex.rs

@@ -181,10 +181,28 @@ pub struct Mutex<T: ?Sized> {
    data: UnsafeCell<T>,
 }

-// these are the only places where `T: Send` matters; all other
-// functionality works fine on a single thread.
+/// A [`Mutex`] is safe to send to other threads by design.


Mmmh, this line seems redundant and conflicts with the rest...

joboet · 2025-01-24T13:47:53Z

library/std/src/sync/poison/mutex.rs

 #[stable(feature = "rust1", since = "1.0.0")]
 unsafe impl<T: ?Sized + Send> Send for Mutex<T> {}
+
+/// [`Mutex`] can be `Sync` if its inner type `T` is `Send`.


I much prefer your phrasing above: "T must be Send" makes the precondition more explicit:

Suggested change

/// [`Mutex`] can be `Sync` if its inner type `T` is `Send`.

/// `T` must be `Send` for [`Mutex`] to be `Sync`.

I think that would also help with understanding the next sentence – my mind currently interprets the "This" as refering to the "Mutex can be Sync" part, not the T: Send part (which is the relevant point).

library/std/src/sync/poison/mutex.rs

joboet · 2025-01-24T14:04:21Z

@rustbot author

ranger-ross · 2025-01-26T12:28:26Z

Sorry for the language-lawyering. I'm studying a humanities subject, so I can be a bit picky about this stuff... 😄

No worries. I was expecting a fairly stringent review for this. 😆
Thanks for the feedback.

I believe I addressed your comments above so let me know what you think. :)

@rustbot reviewer

joboet · 2025-01-29T12:54:53Z

library/std/src/sync/poison/mutex.rs

+/// Also note that it is not necessary for `T` to be `Sync` as it is not possible
+/// to acquire a `&T` from a [`Mutex`].


It is possible to get a &T by dereferencing a mutex guard, so this is misleading. The argument here is that the &T is only made available to one thread at a time (if T is not Sync). This is essentially equivalent to sending a &mut T between threads and creating a &T from that.

Ahh, my bad. I updated this doc based on your comment above. (it was basically a copy/paste since I think your explanation is quiet good)

joboet

This reads very nicely. I still have one concern and one nit, but otherwise, this seems ready to merge.

joboet · 2025-01-29T12:55:41Z

library/std/src/sync/poison/mutex.rs

@@ -211,8 +230,16 @@ pub struct MutexGuard<'a, T: ?Sized + 'a> {
    poison: poison::Guard,
 }

+/// A [`MutexGuard`] is not `Send` to maximize platform portablity.


Suggested change

/// A [`MutexGuard`] is not `Send` to maximize platform portablity.

/// A [`MutexGuard`] is not `Send` to maximize platform portablity.

///

ranger-ross · 2025-02-01T05:24:53Z

@rustbot reviewer

joboet · 2025-02-01T10:43:34Z

Thank you for the PR!
@bors r+ rollup

bors · 2025-02-01T10:43:36Z

📌 Commit 3d84a49 has been approved by joboet

It is now in the queue for this repository.

docs: Documented Send and Sync requirements for Mutex + MutexGuard This an attempt to continue where rust-lang#123225 left off. I did some light clean up from the work done in that PR. I also documented the `!Send` + `Sync` implementations for `MutexGuard` to the best of my knowledge. Let me know if I got anything wrong 😄 fixes rust-lang#122856 cc: `@IoaNNUwU` r? `@joboet`

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#130514 (Implement MIR lowering for unsafe binders) - rust-lang#135684 (docs: Documented Send and Sync requirements for Mutex + MutexGuard) - rust-lang#135760 (Add `unchecked_disjoint_bitor` per ACP373) - rust-lang#136154 (Use +secure-plt for powerpc-unknown-linux-gnu{,spe}) - rust-lang#136309 (set rustc dylib on manually constructed rustc command) - rust-lang#136339 (CompileTest: Add Directives to Ignore `arm-unknown-*` Targets) - rust-lang#136368 (Make comma separated lists of anything easier to make for errors) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#130514 (Implement MIR lowering for unsafe binders) - rust-lang#135684 (docs: Documented Send and Sync requirements for Mutex + MutexGuard) - rust-lang#136307 (Implement all mix/max functions in a (hopefully) more optimization amendable way) - rust-lang#136360 (Stabilize `once_wait`) - rust-lang#136364 (document that ptr cmp is unsigned) - rust-lang#136374 (Add link attribute for Enzyme's LLVMRust FFI) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#135684 - ranger-ross:mutex-docs, r=joboet docs: Documented Send and Sync requirements for Mutex + MutexGuard This an attempt to continue where rust-lang#123225 left off. I did some light clean up from the work done in that PR. I also documented the `!Send` + `Sync` implementations for `MutexGuard` to the best of my knowledge. Let me know if I got anything wrong 😄 fixes rust-lang#122856 cc: ``@IoaNNUwU`` r? ``@joboet``

rustbot assigned joboet Jan 18, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 18, 2025

This comment has been minimized.

Sign in to view

ranger-ross force-pushed the mutex-docs branch from 068039b to 2d34036 Compare January 19, 2025 02:27

joboet requested changes Jan 21, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 21, 2025

ranger-ross force-pushed the mutex-docs branch from 2d34036 to ffb26ec Compare January 22, 2025 13:15

ranger-ross force-pushed the mutex-docs branch from ffb26ec to 162ae1b Compare January 22, 2025 13:27

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 22, 2025

joboet reviewed Jan 24, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 24, 2025

ranger-ross force-pushed the mutex-docs branch from 162ae1b to 3d9c134 Compare January 26, 2025 12:23

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 26, 2025

joboet reviewed Jan 29, 2025

View reviewed changes

joboet requested changes Jan 29, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 29, 2025

docs: Documented Send and Sync requirements for Mutex + MutexGuard

3d84a49

ranger-ross force-pushed the mutex-docs branch from 3d9c134 to 3d84a49 Compare February 1, 2025 05:20

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 1, 2025

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 1, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 1, 2025

matthiaskrgr mentioned this pull request Feb 1, 2025

Rollup of 7 pull requests #136388

Closed

matthiaskrgr mentioned this pull request Feb 1, 2025

Rollup of 6 pull requests #136389

Merged

bors merged commit 9dfdef6 into rust-lang:master Feb 1, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Feb 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Documented Send and Sync requirements for Mutex + MutexGuard #135684

docs: Documented Send and Sync requirements for Mutex + MutexGuard #135684

ranger-ross commented Jan 18, 2025

This comment has been minimized.

joboet left a comment

joboet Jan 21, 2025

joboet Jan 21, 2025

ranger-ross Jan 22, 2025

ranger-ross commented Jan 22, 2025

ranger-ross commented Jan 22, 2025

joboet left a comment

joboet Jan 24, 2025

joboet Jan 24, 2025

joboet commented Jan 24, 2025

ranger-ross commented Jan 26, 2025 •

edited

Loading

joboet Jan 29, 2025

ranger-ross Feb 1, 2025

joboet left a comment

joboet Jan 29, 2025

ranger-ross commented Feb 1, 2025

joboet commented Feb 1, 2025

bors commented Feb 1, 2025

	/// [`Mutex`] can be `Sync` if its inner type `T` is `Send`.
	/// `T` must be `Send` for [`Mutex`] to be `Sync`.

		/// Also note that it is not necessary for `T` to be `Sync` as it is not possible
		/// to acquire a `&T` from a [`Mutex`].

	/// A [`MutexGuard`] is not `Send` to maximize platform portablity.
	/// A [`MutexGuard`] is not `Send` to maximize platform portablity.
	///

docs: Documented Send and Sync requirements for Mutex + MutexGuard #135684

docs: Documented Send and Sync requirements for Mutex + MutexGuard #135684

Conversation

ranger-ross commented Jan 18, 2025

This comment has been minimized.

joboet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranger-ross commented Jan 22, 2025

ranger-ross commented Jan 22, 2025

joboet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joboet commented Jan 24, 2025

ranger-ross commented Jan 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joboet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranger-ross commented Feb 1, 2025

joboet commented Feb 1, 2025

bors commented Feb 1, 2025

ranger-ross commented Jan 26, 2025 •

edited

Loading