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.

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

Example CondVar code should explicitly drop the MutexGuard after it's done with it #67457

Closed
dfoxfranke opened this issue Dec 20, 2019 · 1 comment · Fixed by #73104
Closed
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@dfoxfranke
Copy link

dfoxfranke commented Dec 20, 2019

At the very end of

/// use std::sync::{Arc, Mutex, Condvar};
/// use std::thread;
///
/// let pair = Arc::new((Mutex::new(false), Condvar::new()));
/// let pair2 = pair.clone();
///
/// // Inside of our lock, spawn a new thread, and then wait for it to start.
/// thread::spawn(move|| {
/// let (lock, cvar) = &*pair2;
/// let mut started = lock.lock().unwrap();
/// *started = true;
/// // We notify the condvar that the value has changed.
/// cvar.notify_one();
/// });
///
/// // Wait for the thread to start up.
/// let (lock, cvar) = &*pair;
/// let mut started = lock.lock().unwrap();
/// while !*started {
/// started = cvar.wait(started).unwrap();
/// }
/// ```
and in all similar examples elsewhere in the file, there should be a std::mem::drop(started) call. Leaving a condition variable's mutex held after we're no longer watching it will block and possibly deadlock notifier threads.

@dfoxfranke
Copy link
Author

Real-world example where this bit me: in the main thread of my program, I was waiting on a condition variable and then calling runtime.shutdown_now().wait().unwrap(), with runtime being a tokio runtime. The condition variable was set by a SIGTERM signal handler. Worked fine, unless the program got two back-to-back signals without the main thread waking up in between. In that case, the second instance of the signal handler would block on the mutex, which would never get released because the main thread was in turn blocked inside wait() waiting for the signal handler future's poll() method to return.

@Centril Centril added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 21, 2019
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 14, 2020
…ample, r=dtolnay

Example about explicit mutex dropping

Fixes rust-lang#67457.

Following the remarks made in rust-lang#73074, I added an example on the main `Mutex` type, with a situation where there is mutable data and a computation result.

In my testing it is effectively needed to explicitly drop the lock, else it deadlocks.

r? @dtolnay because you were the one to review the previous PR.
RalfJung added a commit to RalfJung/rust that referenced this issue Jun 15, 2020
…ample, r=dtolnay

Example about explicit mutex dropping

Fixes rust-lang#67457.

Following the remarks made in rust-lang#73074, I added an example on the main `Mutex` type, with a situation where there is mutable data and a computation result.

In my testing it is effectively needed to explicitly drop the lock, else it deadlocks.

r? @dtolnay because you were the one to review the previous PR.
@bors bors closed this as completed in 7c8b941 Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
2 participants