Add shared mutable state documentation #1759
Conversation
|
Maybe we should link to tokio's "Which kind of mutex should you use?" docs though? |
|
Yeah that'd be fine with me. |
|
@ThanePatrol check the referenced pull request I crested #1777. |
|
@Vagelis-Prokopiou Thanks for letting me know. I've shared the repo with you and incorporated your changes. |
The documentation for sharing a mutable state did not provide examples. Wrote explanation of the steps required to share mutable states between routes. Provided an example in code. This was inspired from discussion tokio-rs#629 tokio-rs#629
The cost of using an async mutex was not mentioned. Rather than defaulting to async, the documentation now gives the std Mutex in the example and links to the discussion from tokio.
Example was opinionated and specific. Also, changed written description to be more general.
Provides more consistent style across code base
Vagelis had a similar pull request. Changes were merged. Added a note about deadlocks. A detailed discussion may not be appropriate. However, a note that makes people aware of the potential consequences could help reduce pitfalls of share mutable state.
Vagelis-Prokopiou
force-pushed
the
state-documentation-addition
branch
from
e95ba1b
to
7ebccbb
Compare
|
This PR looks ready. Kindly waiting on the owners @davidpdrsn @jplatte. |
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
|
@jplatte suggested change applied. |
|
I had lots of nitpicky suggestions so I've rewritten it myself. @jplatte wanna give it a final look? |
davidpdrsn
changed the title
| /// #[derive(Clone)] | ||
| /// struct AppState { | ||
| /// data: Arc<Mutex<String>>, | ||
| /// } |
There was a problem hiding this comment.
I'm a little worried that people will do fine-grained locking where more coarse locking is the right solution, based on this example. I'm not sure just showing an Arc<Mutex<_>> with everything inside locked at once is better either though...
There was a problem hiding this comment.
@jplatte Maybe something like the following can be added, in order to make your point super explicit?
#[derive(Clone)]
struct AppState {
// This is just an example usage, not a best practice.
// Choose a setup that matches your specific use case.
data: Arc<Mutex<String>>,
}
There was a problem hiding this comment.
I'm a little worried that people will do fine-grained locking where more coarse locking is the right solution, based on this example.
Yeah I'm a little worried about that as well. But like you say putting a mutex around the whole state probably isn't right either 😅 I dunno. Think I'll just keep it as is.
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Motivation
I spent a lot of time trying to find documentation for creating a mutable state. I eventually found discussion #629
Solution
Added documentation for creating mutable state and wrote a small trivial example using a
Vec<String>