Fix mutex's docs inconsistency #40608

GuillaumeGomez · 2017-03-17T17:32:52Z

r? @steveklabnik
cc @rust-lang/docs

ScottAbbey · 2017-03-17T17:35:58Z

src/libstd/sync/mutex.rs

    /// held. An RAII guard is returned to allow scoped unlock of the lock. When
-    /// the guard goes out of scope, the mutex will be unlocked.
+    /// the guard goes out of scope, the mutex's lock will be unlocked.


the mutex's lock will be unlocked.

I'm not sure this change is an improvement over the original. Shouldn't it match the language used in line 27:

ever accessed when the mutex is locked.

I agree - a mutex doesn't contain a lock, it is a lock, IMO.

ScottAbbey · 2017-03-17T18:06:08Z

src/libstd/sync/mutex.rs

@@ -183,18 +183,18 @@ impl<T: ?Sized> Mutex<T> {
    /// Acquires a mutex, blocking the current thread until it is able to do so.


Acquires a mutex, blocking

Two lines after this, the comment speaks of acquiring "the lock," should this instance be modified as well?

Concern here: "lock" is generic, while "mutex" is shorthand for "mutually exclusive lock" and therefore seems most specific and correct to me.

This is the same paragraph directly quoted in #40176, would you say it would have been better to change the one instance of "the lock" to "the mutex" instead of the other way around?

Yes "lock" or "acquire" as the verb, and "mutex" as the noun. Therefore "the mutex" makes more sense than "the lock". 😃

I agree with @ScottAbbey on this one. For non-english speaker, acquiring a lock better than acquiring a mutex which doesn't mean much.

@GuillaumeGomez while I agree, this isn't just pros. We're discussing technical documentation and as such should be as specific as possible, without being exclusionary.

If we're going with the general term, then we need to the MSDN thing of explaining that the lock is exclusive and will block any threads that attempt to acquire the lock while the lock is held.

IMO this gets verbose, but does have the advantage of being very approachable and lends itself well to education as well as to translation.

frewsxcv · 2017-03-18T15:17:37Z

cc @rust-lang/libs for terminology here

alexcrichton · 2017-03-18T16:25:12Z

I personally consider "mutex" and "lock" as interchangeable, but canonicalizing on one seems fine (e.g. they're interchangeable, so doesn't matter what we call it!)

ScottAbbey · 2017-03-19T03:48:23Z

I agree with @whoisj, almost every reference to "lock" should instead be "mutex."

The language that seems to make the most sense is:

The mutex may be locked or unlocked.

The original complaint in #40176 was that the lock and the mutex were both used to refer to the Mutex object. By changing all references of "mutex" to "lock", the problem only got worse, now we are talking about locking and unlocking the lock.

Searching for some other published writing about a similar topic, I found the following in Chapter 30 (page 634) of The Linux Programming Interface, by Michael Kerrisk:

When a thread locks a mutex, it becomes the owner of that mutex. Only the mutex owner can unlock the mutex.

I find this short quote easy to read and understand. Rewriting that sentence to fit the state of this mutex documentation as it currently stands, we would have something like:

When a thread acquires a lock, it becomes the holder of that lock. Only the lock owner can unlock the mutex.

This is still in the same confusing state we started with. Shouldn't the thread acquire (or lock) "the mutex", be the holder (or owner) of "the mutex", and "the mutex holder (or owner)" be able to unlock it?

There are still some consistency issues. In the comment for is_poisoned:

/// Determines whether the lock is poisoned
...
/// panic!(); // the mutex gets poisoned

The first one and several other "lock is poisoned" references should instead say "the mutex".

This phrase appears many times:

/// If another user of this mutex panicked while holding the lock, then this call will return an error instead.

I think all of those should be changed back to "while holding the mutex".

Under struct Mutex<T>:

to ensure that the native mutex is used correctly we box the inner lock...

Should be "the inner mutex".

I don't trust myself to try writing any of this as I really don't know what I'm talking about anyway.

steveklabnik · 2017-03-20T15:27:26Z

agree wholeheartedly with @ScottAbbey

steveklabnik · 2017-03-31T18:20:55Z

@GuillaumeGomez ping! any updates on this?

GuillaumeGomez · 2017-03-31T19:00:22Z

I need to update it. Will do it in the day.

GuillaumeGomez · 2017-03-31T19:10:23Z

Ok so I made a first update. Did I miss anything?

steveklabnik

a couple more spots, then r=me

steveklabnik · 2017-04-03T15:43:46Z

src/libstd/sync/mutex.rs

@@ -92,7 +92,7 @@ use sys_common::poison::{self, TryLockError, TryLockResult, LockResult};
 /// let lock2 = lock.clone();
 ///
 /// let _ = thread::spawn(move || -> () {
-///     // This thread will acquire the mutex first, unwrapping the result of
+///     // This thread will acquire the lock first, unwrapping the result of


this is the inverse

steveklabnik · 2017-04-03T15:43:53Z

src/libstd/sync/mutex.rs

@@ -180,10 +180,10 @@ impl<T> Mutex<T> {
 }

 impl<T: ?Sized> Mutex<T> {
-    /// Acquires a mutex, blocking the current thread until it is able to do so.
+    /// Acquires a lock, blocking the current thread until it is able to do so.


steveklabnik · 2017-04-03T15:43:58Z

src/libstd/sync/mutex.rs

    ///
    /// This function will block the local thread until it is available to acquire
-    /// the mutex. Upon returning, the thread is the only thread with the mutex
+    /// the lock. Upon returning, the thread is the only thread with the lock


steveklabnik · 2017-04-03T15:44:04Z

src/libstd/sync/mutex.rs

-    /// If another user of this mutex panicked while holding the mutex, then
-    /// this call will return an error once the mutex is acquired.
+    /// If another user of this mutex panicked while holding the lock, then
+    /// this call will return an error once the lock is acquired.


same on both lines

steveklabnik · 2017-04-03T15:44:10Z

src/libstd/sync/mutex.rs

-    /// If another user of this mutex panicked while holding the mutex, then
-    /// this call will return failure if the mutex would otherwise be
+    /// If another user of this mutex panicked while holding the lock, then
+    /// this call will return failure if the lock would otherwise be


GuillaumeGomez · 2017-04-03T16:58:03Z

I updated to @steveklabnik's comments.

steveklabnik · 2017-04-03T17:03:22Z

@bors: r+ rollup

bors · 2017-04-03T17:03:23Z

📌 Commit e7c2160 has been approved by steveklabnik

@steveklabnik