Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a comment about locking a Mutex multiple times #33034

Merged
merged 1 commit into from
Apr 17, 2016
Merged

Add a comment about locking a Mutex multiple times #33034

merged 1 commit into from
Apr 17, 2016

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Apr 16, 2016

Fixes #32260.

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@alexcrichton
Copy link
Member

A guarantee that we can provide here is that a recursive call to lock will never return (as that wouldn't be sound otherwise). We could then perhaps just note that this is implemented via deadlocking on some platforms and panicking on others?

@tbu- tbu- force-pushed the pr_doc_mutex_lock branch from 35d0929 to 26cc9cd Compare April 16, 2016 22:43
@tbu-
Copy link
Contributor Author

tbu- commented Apr 16, 2016

@alexcrichton Updated.

nagisa
nagisa reviewed Apr 17, 2016
View reviewed changes
src/libstd/sync/mutex.rs
@@ -205,10 +205,19 @@ impl<T: ?Sized> Mutex<T> {
/// 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 exact behavior on locking a mutex twice in the same thread is left
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“locking a mutex twice” is ambiguous in whether 2nd lock happens with mutex being locked or unlocked.

I would reword this as “behaviour of locking the mutex in the thread which already holds the lock is left unspecified” instead.

@tbu- tbu- force-pushed the pr_doc_mutex_lock branch from 26cc9cd to df748bf Compare April 17, 2016 19:31
@tbu-
Copy link
Contributor Author

tbu- commented Apr 17, 2016

@nagisa Sounds good. Changed.

@nagisa
Copy link
Member

nagisa commented Apr 17, 2016

@bors r+ rollup df748bf

Seems good

@bors
Copy link
Contributor

bors commented Apr 17, 2016

📌 Commit df748bf has been approved by nagisa

@bors
Copy link
Contributor

bors commented Apr 17, 2016

⌛ Testing commit df748bf with merge ed7c567...

bors added a commit that referenced this pull request Apr 17, 2016
@bors
Auto merge of #33034 - tbu-:pr_doc_mutex_lock, r=nagisa
ed7c567 
Add a comment about locking a `Mutex` multiple times

Fixes #32260.
@bors
Copy link
Contributor

bors commented Apr 17, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-64-x-armhf, auto-linux-64-x-armsf, auto-linux-64-x-freebsd, auto-linux-64-x-netbsd, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild

@bors bors merged commit df748bf into rust-lang:master Apr 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@aturon aturon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tbu- @rust-highfive @alexcrichton @nagisa @bors @aturon