-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Doc of mutex advising std blocking mutex look totally misleading #5024
Comments
Whether or not something counts as blocking has to do with the duration it takes. Usually, you will be blocking on the mutex for a sufficiently short time that you avoid problems. |
First time I read this, thus that mostly true in practice in theory that false, I think that asking for trouble. I think use mutex (as a user) in async is often a mistake, you better use channel and co that will do the unsafe job for you. I have trouble to understand exactly the doc:
The doc mix std and tokio mutex implementation is hard to follow, and I don't understand how tokio mutex would not deadlock like std mutex since the doc say "keep it locked across an .await point." for both mutex. It's look like the doc should be separate in 3 sections:
BTW: should the same advice of using RwLock from std instead of tokio RwLock apply ? The doc of RwLock currently doesn't at all suggest use std RwLock. |
I'm sure this particular section in the documentation could be improved, and I have a draft for a article that discusses how the std mutex is used in more detail, but I haven't had the time to work on it for a while. The Tokio mutex doesn't deadlock when held across an Yes, it also applies to |
As I understand it, the first sentence is a direct contradiction of the rest. I think it's also incorrect, because this works fine: #[tokio::main]
async fn main() {
let m = std::sync::Mutex::new(42usize);
let l = m.lock().unwrap();
tokio::time::sleep(core::time::Duration::from_secs(1)).await;
drop(l);
println!("Hello, world!");
} The rest of the explanation is really vague. An example for why and how "this virtually never leads to correct concurrent code in practice", would be great. |
The code you've shared is fine because there's only one user of the mutex. But if you had two async tasks running at the same time executing that code on the same mutex, then it would deadlock. (Or at least, it is very likely to.) |
To make my point clearer: Yes, there exist extremely simple programs where holding an std mutex locked over an await does not deadlock. Typically that is because you never have two tasks call |
Ok, thank you! I managed to come up with a simple example that breaks with std's Mutex and works with tokio's. This one uses std::sync::Mutex and blocks forever use std::sync::Mutex;
#[tokio::main]
async fn main() {
let m = Box::leak(Box::new(Mutex::new(1i32)));
let t1 = task_1(m);
let t2 = task_2(m);
tokio::join!(t1, t2);
}
async fn task_1 (m : &Mutex<i32>) {
let l = m.lock().unwrap();
tokio::time::sleep(core::time::Duration::from_secs(2)).await;
drop(l);
println!("task_1 done");
}
async fn task_2 (m : &Mutex<i32>) {
tokio::time::sleep(core::time::Duration::from_secs(2)).await;
let _unused = m.lock().unwrap(); // this will never return, because the runtime is stuck here instead of returning to task_1's sleep await point
drop(_unused);
println!("task_1 done");
} This one uses tokio's and works, because lock() in task_2 does not block the scheduler and tokio can continue running task_1. Therefore, task_1 can actually cross its await point and release the lock, allowing task_2 to run. use tokio::sync::Mutex;
#[tokio::main]
async fn main() {
let m = Box::leak(Box::new(Mutex::new(1i32)));
let t1 = task_1(m);
let t2 = task_2(m);
tokio::join!(t1, t2);
}
async fn task_1 (m : &Mutex<i32>) {
let l = m.lock().await;
tokio::time::sleep(core::time::Duration::from_secs(2)).await;
drop(l);
println!("task_1 done");
}
async fn task_2 (m : &Mutex<i32>) {
tokio::time::sleep(core::time::Duration::from_secs(2)).await;
let _unused = m.lock().await;
drop(_unused);
println!("task_1 done");
} |
So, as I understand it now, that first sentence ("The feature that the async mutex offers over the blocking mutex is the ability to keep it locked across an .await point.") could be rephrased as "The feature that the async mutex offers over the blocking mutex is the ability to be locked over an await point without deadlocking the executor on concurrent attempts to lock the same mutex". Now, since the docs also say "is ok and often preferred to use the ordinary Mutex from the standard library in asynchronous code", I wonder what the "correct" use case for std's mutex in async code is. Sharing something between sync code and async code? |
Doc of mutex currently state:
That contrary to all my understanding and all the doc of async I know. Tokio doc is very clear about a task should not block. Waiting on a std mutex is blocking, I don't see how the doc can suggest to use std mutex in async context. At the very least it should state that the user should use block_in_place
What is the rational that it's ok to block a async task but only for mutex of std ?
The text was updated successfully, but these errors were encountered: