4 ms·

Presumably the author is using tokio which requires the future constructed (e.g the async function) to be Send (either because of the rules of Rust or annotated

Presumably the author is using tokio which requires the future constructed (e.g the async function) to be Send (either because of the rules of Rust or annotated as Send) since tokio is a work-stealing runtime and any thread might end up executing a given future (or even start executing and then during a pause move it for completion on another thread). std::sync::MutexGuard intentionally isn't annotated with Send because there are platforms that require the acquiring thread be the one to unlock the mutex. One caveat though - using a normal std Mutex within an async environment is an antipattern and should not be done - you can cause all sorts of issues & I believe even deadlock your entire code. You should be using tokio sync primitives (e.g. tokio Mutex) which can yield to the reactor when it needs to block. Otherwise the thread that's running the future blocks forever waiting for that mutex and that reactor never does anything else which isn't how tokio is designed). So the compiler is warning about 1 problem, but you also have to know to be careful to know not to call blocking functions in an async function.

maplant 1y ago
> using a normal std Mutex within an async environment is an antipattern and should not be done This is simply not true, and the tokio documentation says as much: "Contrary to popular belief, it is ok and often preferred to use the ordinary Mutex from the standard library in asynchronous code." https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html#which-kind-of-mutex-should-you-use https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html#wh...
- deleted 1y ago
  [deleted]
- vlovich123 1y ago
  I think that’s a dangerous recommendation and ignores the consequence of doing this within a single threaded reactor (your app will hang permanently if the lock is contended) or assumes you won’t have N threads trying to acquire this lock where N is the size of your thread pool which is entirely possible on a heavily accessed server if the core state that needs to be accessed by every request is locked with said mutex.
  - maplant 1y ago
    If you're using a single threaded reactor you don't need a mutex at all; a Rc<RefCell<_>> will do just fine. And if you want other tasks to yield until you're done holding the borrow, the solution is simple: don't await until you're done. https://docs.rs/tokio/latest/tokio/task/struct.LocalSet.html#awaiting-a-localset https://docs.rs/tokio/latest/tokio/task/struct.LocalSet.html... there are absolutely situations where tokio's mutex and rwlock are useful, but the vast majority of the time you shouldn't need them
    - vlovich123 1y ago
      As you know Rc<RefCell> won't work for a data structure that's being accessed by other threads (e.g. spawn_blocking or explicitly accessed threads). Just because you are using the single-threaded reactor does not mean you don't have other threads to access the data.
      - maplant 1y ago
        It seems like you've found a very specific situation where tokio's Mutex is helpful but it is simply not common to be structuring an async rust application with a single threaded reactor and copious uses of spawn blocking. The advice to go for the std Mutex first is generally good
dilawar 1y ago
> One caveat though - using a normal std Mutex within an async environment is an antipattern and should not be done - you can cause all sorts of issues & I believe even deadlock your entire code. True. I used std::mutex with tokio and after a few days my API would not respond unless I restarted the container. I was under the impression that if it compiles, it's gonna just work (fearless concurrency) which is usually the case.
- deleted 1y ago
  [deleted]
- maplant 1y ago
  It sounds like you have a deadlock somewhere in your code and it's unlikely that the choice of Mutex really fixed that
  - vlovich123 1y ago
    Or you do something super expensive while holding the mutex. Or your server is heavily contended enough that all worker threads are blocked on this mutex and no reactor can make forward progress.
Spivak 1y ago
Thank you! It seems so simple in hindsight to have a type that means "can be moved to another thread safely." But the fact that using a Mutex in your function changes the "type" is really novel. It becoming Send once the lock is released before await is just fantastic.
- VWWHFSfQ 1y ago
  Rust's marker traits (Send, Sync, etc.) + RAII is really what makes it so magical to me.
- NobodyNada 1y ago
  The way that this fully works together is: - The return type of Mutex::lock() is a MutexGuard, which is a smart pointer type that 1) implements Deref so it can be dereferenced to access the underlying data, 2) implements Drop to unlock the mutex when the guard goes out of scope, and 3) implements !Send so the compiler knows it is unsafe to send between threads: https://doc.rust-lang.org/std/sync/struct.MutexGuard.html https://doc.rust-lang.org/std/sync/struct.MutexGuard.html - Rust's implementation of async/await works by transforming an async function into a state machine object implementing the Future trait. The compiler generates an enum that stores the current state of the state machine and all the local variables that need to live across yield points, with a poll function that (synchronously) advances the coroutine to the next yield point: https://doc.rust-lang.org/std/future/trait.Future.html https://doc.rust-lang.org/std/future/trait.Future.html - In Rust, a composite type like a struct or enum automatically implements Send if all of its members implement Send. - An async runtime that can move tasks between threads requires task futures to implement Send. So, in the example here: because the author held a lock across an await point, the compiler must store the MutexGuard smart pointer as a field of the Future state machine object. Since MutexGuard is !Send, the future also is !Send, which means it cannot be used with an async runtime that moves tasks between threads. If the author releases the lock (i.e. drops the lock guard) before awaiting, then the guard does not live across yield points and thus does not need to be persisted as part of the state machine object -- it will be created and destroyed entirely within the span of one call to Future::poll(). Thus, the future object can be Send, meaning the task can be migrated between threads.
rcxdude 1y ago
It can be an issue, but only if you have long enough contention periods for the mutex to matter. A Mutex which is only held for short time periouds can work perfectly fine in an async context. And it's only going to cause deadlocks if it would cause deadlocks in a threaded context, since if Mutexes can only be held between yield points then it'll only ever be running tasks that are contending for them.
- vlovich123 1y ago
  Imagine if every thread tries to acquire this mutex on every HTTP request. And then you also lock it in a spawn_blocking. On a heavily contended lock, all the reactor threads will end up stuck waiting for the spawn_blocking to finish and the service will appear hung since even requests that don’t even need that lock can’t get processed. And if spawn_blocking is doing synchronous network I/O or reading a file from a hung NFS mount, you’re completely hosed.
  - rcxdude 1y ago
    You would be in a similarly bad situation if such a lock was async as well. You want to keep locks as short as possible, regardless, and a hung task with a lock is generally going to propagate issues throughout the system until it's dealt with.
    - vlovich123 1y ago
      If you have an async lock that can be held across await points, your motivation to spawn_block decreases. Additionally, even though the long running lock would be a problem, APIs that aren't taking that lock continue to work vs the entire HTTP service being hung which is a huge difference (e.g. health monitoring would time out making the service seem unresponsive vs 1 highly contended task just waiting on a lock).
      - rcxdude 1y ago
        >e.g. health monitoring would time out making the service seem unresponsive vs 1 highly contended task just waiting on a lock If anything that's a disadvantage. You want your health monitoring to be the canary, not something that keeps on trucking even if the system is no longer doing useful work. (See the classic safety critical software fail of 'I need a watchdog... I'll just feed it regularly in an isolated task')
sunshowers 1y ago
Using a Tokio mutex is even more of an antipattern :) come to my RustConf talk about async cancellation next week to find out why!
- vlovich123 1y ago
  Most people on this forum are not attending RustConf. It might be helpful to post at least the abstract of your idea.
  - sunshowers 1y ago
    The big thing is that futures are passive, so any future can be cancelled at any await point by dropping it or not polling it any more. So if you have a situation like this, which in my experience is a very common way to use mutexes: let guard = mutex.lock().await; // guard.data is Option<T>, Some to begin with let data = guard.data.take(); // guard.data is now None let new_data = process_data(data).await; guard.data = Some(new_data); // guard.data is Some again Then you could cancel the future at the await point in between while the lock is held, and as a result guard.data will not be restored to Some.
    - neandrake 1y ago
      Same would be true for any resource that needs cleaned up, right? Referring to stop-polling-future as canceling is probably not good nomenclature. Typically canceling some work requires cleanup, if only to be graceful let alone properly releasing resources.
      - sunshowers 1y ago
        Yes, this is true of any resource. But Tokio mutexes, being shared mutable state, are inherently likely to run into bugs in production. In the Rust community, cancellation is pretty well-established nomenclature for this. Hopefully the video of my talk will be up soon after RustConf, and I'll make a text version of it as well for people that prefer reading to watching.
benmmurphy 1y ago
original poster was 'lucky' that he was using a work stealing engine. if the engine was not moving tasks between threads then i think the compiler would have been happy and he could have had the fun of debugging what happens when the same thread tries to lock the same mutex twice. the rust compiler won't save you from this kind of bug. > The exact behavior on locking a mutex in the thread which already holds the lock is left unspecified. However, this function will not return on the second call (it might panic or deadlock, for example).

maplant 1y ago

> using a normal std Mutex within an async environment is an antipattern and should not be done This is simply not true, and the tokio documentation says as much: "Contrary to popular belief, it is ok and often preferred to use the ordinary Mutex from the standard library in asynchronous code." https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html#which-kind-of-mutex-should-you-use https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html#wh...

deleted 1y ago
[deleted]
vlovich123 1y ago
I think that’s a dangerous recommendation and ignores the consequence of doing this within a single threaded reactor (your app will hang permanently if the lock is contended) or assumes you won’t have N threads trying to acquire this lock where N is the size of your thread pool which is entirely possible on a heavily accessed server if the core state that needs to be accessed by every request is locked with said mutex.
- maplant 1y ago
  If you're using a single threaded reactor you don't need a mutex at all; a Rc<RefCell<_>> will do just fine. And if you want other tasks to yield until you're done holding the borrow, the solution is simple: don't await until you're done. https://docs.rs/tokio/latest/tokio/task/struct.LocalSet.html#awaiting-a-localset https://docs.rs/tokio/latest/tokio/task/struct.LocalSet.html... there are absolutely situations where tokio's mutex and rwlock are useful, but the vast majority of the time you shouldn't need them
  - vlovich123 1y ago
    As you know Rc<RefCell> won't work for a data structure that's being accessed by other threads (e.g. spawn_blocking or explicitly accessed threads). Just because you are using the single-threaded reactor does not mean you don't have other threads to access the data.
    - maplant 1y ago
      It seems like you've found a very specific situation where tokio's Mutex is helpful but it is simply not common to be structuring an async rust application with a single threaded reactor and copious uses of spawn blocking. The advice to go for the std Mutex first is generally good

dilawar 1y ago

> One caveat though - using a normal std Mutex within an async environment is an antipattern and should not be done - you can cause all sorts of issues & I believe even deadlock your entire code. True. I used std::mutex with tokio and after a few days my API would not respond unless I restarted the container. I was under the impression that if it compiles, it's gonna just work (fearless concurrency) which is usually the case.

deleted 1y ago
[deleted]
maplant 1y ago
It sounds like you have a deadlock somewhere in your code and it's unlikely that the choice of Mutex really fixed that
- vlovich123 1y ago
  Or you do something super expensive while holding the mutex. Or your server is heavily contended enough that all worker threads are blocked on this mutex and no reactor can make forward progress.

Spivak 1y ago

Thank you! It seems so simple in hindsight to have a type that means "can be moved to another thread safely." But the fact that using a Mutex in your function changes the "type" is really novel. It becoming Send once the lock is released before await is just fantastic.

VWWHFSfQ 1y ago
Rust's marker traits (Send, Sync, etc.) + RAII is really what makes it so magical to me.
NobodyNada 1y ago
The way that this fully works together is: - The return type of Mutex::lock() is a MutexGuard, which is a smart pointer type that 1) implements Deref so it can be dereferenced to access the underlying data, 2) implements Drop to unlock the mutex when the guard goes out of scope, and 3) implements !Send so the compiler knows it is unsafe to send between threads: https://doc.rust-lang.org/std/sync/struct.MutexGuard.html https://doc.rust-lang.org/std/sync/struct.MutexGuard.html - Rust's implementation of async/await works by transforming an async function into a state machine object implementing the Future trait. The compiler generates an enum that stores the current state of the state machine and all the local variables that need to live across yield points, with a poll function that (synchronously) advances the coroutine to the next yield point: https://doc.rust-lang.org/std/future/trait.Future.html https://doc.rust-lang.org/std/future/trait.Future.html - In Rust, a composite type like a struct or enum automatically implements Send if all of its members implement Send. - An async runtime that can move tasks between threads requires task futures to implement Send. So, in the example here: because the author held a lock across an await point, the compiler must store the MutexGuard smart pointer as a field of the Future state machine object. Since MutexGuard is !Send, the future also is !Send, which means it cannot be used with an async runtime that moves tasks between threads. If the author releases the lock (i.e. drops the lock guard) before awaiting, then the guard does not live across yield points and thus does not need to be persisted as part of the state machine object -- it will be created and destroyed entirely within the span of one call to Future::poll(). Thus, the future object can be Send, meaning the task can be migrated between threads.

rcxdude 1y ago

It can be an issue, but only if you have long enough contention periods for the mutex to matter. A Mutex which is only held for short time periouds can work perfectly fine in an async context. And it's only going to cause deadlocks if it would cause deadlocks in a threaded context, since if Mutexes can only be held between yield points then it'll only ever be running tasks that are contending for them.

vlovich123 1y ago
Imagine if every thread tries to acquire this mutex on every HTTP request. And then you also lock it in a spawn_blocking. On a heavily contended lock, all the reactor threads will end up stuck waiting for the spawn_blocking to finish and the service will appear hung since even requests that don’t even need that lock can’t get processed. And if spawn_blocking is doing synchronous network I/O or reading a file from a hung NFS mount, you’re completely hosed.
- rcxdude 1y ago
  You would be in a similarly bad situation if such a lock was async as well. You want to keep locks as short as possible, regardless, and a hung task with a lock is generally going to propagate issues throughout the system until it's dealt with.
  - vlovich123 1y ago
    If you have an async lock that can be held across await points, your motivation to spawn_block decreases. Additionally, even though the long running lock would be a problem, APIs that aren't taking that lock continue to work vs the entire HTTP service being hung which is a huge difference (e.g. health monitoring would time out making the service seem unresponsive vs 1 highly contended task just waiting on a lock).
    - rcxdude 1y ago
      >e.g. health monitoring would time out making the service seem unresponsive vs 1 highly contended task just waiting on a lock If anything that's a disadvantage. You want your health monitoring to be the canary, not something that keeps on trucking even if the system is no longer doing useful work. (See the classic safety critical software fail of 'I need a watchdog... I'll just feed it regularly in an isolated task')

sunshowers 1y ago

Using a Tokio mutex is even more of an antipattern :) come to my RustConf talk about async cancellation next week to find out why!

vlovich123 1y ago
Most people on this forum are not attending RustConf. It might be helpful to post at least the abstract of your idea.
- sunshowers 1y ago
  The big thing is that futures are passive, so any future can be cancelled at any await point by dropping it or not polling it any more. So if you have a situation like this, which in my experience is a very common way to use mutexes: let guard = mutex.lock().await; // guard.data is Option<T>, Some to begin with let data = guard.data.take(); // guard.data is now None let new_data = process_data(data).await; guard.data = Some(new_data); // guard.data is Some again Then you could cancel the future at the await point in between while the lock is held, and as a result guard.data will not be restored to Some.
  - neandrake 1y ago
    Same would be true for any resource that needs cleaned up, right? Referring to stop-polling-future as canceling is probably not good nomenclature. Typically canceling some work requires cleanup, if only to be graceful let alone properly releasing resources.
    - sunshowers 1y ago
      Yes, this is true of any resource. But Tokio mutexes, being shared mutable state, are inherently likely to run into bugs in production. In the Rust community, cancellation is pretty well-established nomenclature for this. Hopefully the video of my talk will be up soon after RustConf, and I'll make a text version of it as well for people that prefer reading to watching.

benmmurphy 1y ago

original poster was 'lucky' that he was using a work stealing engine. if the engine was not moving tasks between threads then i think the compiler would have been happy and he could have had the fun of debugging what happens when the same thread tries to lock the same mutex twice. the rust compiler won't save you from this kind of bug. > The exact behavior on locking a mutex in the thread which already holds the lock is left unspecified. However, this function will not return on the second call (it might panic or deadlock, for example).