Skip to content

Commit f583e82

Browse files
authored
Merge pull request #148 from rylev/locks
Add Alan thinks he needs async locks
2 parents 0be64d5 + 0f03848 commit f583e82

File tree

2 files changed

+132
-4
lines changed

2 files changed

+132
-4
lines changed
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
# 😱 Status quo stories: Alan thinks he needs async locks
2+
3+
## 🚧 Warning: Draft status 🚧
4+
5+
This is a draft "status quo" story submitted as part of the brainstorming period. It is derived from real-life experiences of actual Rust users and is meant to reflect some of the challenges that Async Rust programmers face today.
6+
7+
If you would like to expand on this story, or adjust the answers to the FAQ, feel free to open a PR making edits (but keep in mind that, as they reflect peoples' experiences, status quo stories [cannot be wrong], only inaccurate). Alternatively, you may wish to [add your own status quo story][htvsq]!
8+
9+
## The story
10+
11+
One of Alan's first Rust related tasks in his job at YouBuy is writing an HTTP based service. This service is a simple internal proxy router that inspects an incoming HTTP request and picks the downstream service to call based on certain aspects of the HTTP request.
12+
13+
Alan decides that he'll simply use some shared state that request handlers can read from in order to decide how to proxy the request.
14+
15+
Alan, having read the Rust book and successfully completed the challenge in the [last chapters](https://doc.rust-lang.org/book/ch20-02-multithreaded.html), knows that shared state can be achieved in Rust with reference counting (using `std::sync::Arc`) and locks (using `std::sync::Mutex`). Alan starts by throwing his shared state (a `std::collections::HashMap<String, url::Url>`) into an `Arc<Mutex<T>>`.
16+
17+
Alan, smitten with how quickly he can write Rust code, ends up with some code that compiles that looks roughly like this:
18+
19+
```rust
20+
#[derive(Clone)]
21+
struct Proxy {
22+
routes: Arc<Mutex<HashMap<String, String>>,
23+
}
24+
25+
impl Proxy {
26+
async fn handle(&self, key: String, request: Request) -> crate::Result<Response> {
27+
let routes = self.state.lock().unwrap();
28+
let route = routes.get(key).unwrap_or_else(crate::error::MissingRoute)?;
29+
Ok(self.client.perform_request(route, request).await?)
30+
}
31+
}
32+
```
33+
34+
Alan is happy that his code seems to be compiling! The short but hard learning curve has been worth it. He's having fun now!
35+
36+
Unfortunately, Alan's happiness soon comes to end as he starts integrating his request handler into calls to `tokio::spawn` which he knows will allow him to manage multiple requests at a time. The error message is somewhat cryptic, but Alan is confident he'll be able to figure it out:
37+
38+
```
39+
189 | tokio::spawn(async {
40+
| ^^^^^^^^^^^^ future created by async block is not `Send`
41+
::: /home/alan/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.5.0/src/task/spawn.rs:129:21
42+
|
43+
129 | T: Future + Send + 'static,
44+
| ---- required by this bound in `tokio::spawn`
45+
46+
note: future is not `Send` as this value is used across an await
47+
--> src/handler.rs:787:9
48+
|
49+
786 | let routes = self.state.lock().unwrap();
50+
| - has type `std::sync::MutexGuard<'_, HashMap<String, Url>>` which is not `Send`
51+
787 | Ok(self.client.perform_request(route, request).await?)
52+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `routes` maybe used later
53+
788 | })
54+
| - `routes` is later dropped here
55+
```
56+
57+
Alan stops and takes a deep breath. He tries his best to make sense of the error message. He sort of understands the issue the compiler is telling him. Apparently `routes` is not marked as `Send`, and because it is still alive over a call to `await`, it is making the future his handler returns not `Send`. And tokio's `spawn` function seems to require that the future it received be `Send`.
58+
59+
Alan reaches the boundaries of his knowledge of Rust, so he reaches out over chat to ask his co-worker Barbara for help. Not wanting to bother her, Alan provides the context he's already figured out for himself.
60+
61+
Barbara knows that mutex guards are not `Send` because sending mutex guards to different threads is not a good idea. She suggests looking into async locks which can be held across await points because they are `Send`. Alan looks into the tokio documentation for more info and is easily able to move the use of the standard library's mutex to tokio's mutex. It compiles!
62+
63+
Alan ships his code and it gets a lot of usage. After a while, Alan notices some potential performance issues. It seems his proxy handler does not have the throughput he would expect. Barbara, having newly joined his team, sits down with him to take a look at potential issues. Barbara is immediately worried by the fact that the lock is being held much longer than it needs to be. The lock only needs to be held while accessing the route and not during the entire duration of the downstream request.
64+
65+
She suggests to Alan to switch to not holding the lock across the I/O operations. Alan first tries to do this by explicitly cloning the url and dropping the lock before the proxy request is made:
66+
67+
```rust
68+
impl Proxy {
69+
async fn handle(&self, key: String, request: Request) -> crate::Result<Response> {
70+
let routes = self.state.lock().unwrap();
71+
let route = routes.get(key).unwrap_or_else(crate::error::MissingRoute)?.clone();
72+
drop(routes);
73+
Ok(self.client.perform_request(route, request).await?)
74+
}
75+
}
76+
```
77+
78+
This compiles fine and works in testing! After shipping to production, they notice a large increase in throughput. It seems their change made a big difference. Alan is really excited about Rust, and wants to write more!
79+
80+
Alan continues his journey of learning even more about async Rust. After some enlightening talks at the latest RustConf, he decides to revisit the code that he and Barbara wrote together. He asks himself, is using an *async* lock the right thing to do? This lock should only be held for a very short amount of time. Yielding to the runtime is likely more expensive than just synchronously locking. But he remembers vaguely hearing that you should never use blocking code in async code as this will block the entire async executor from being able to make progress, so he doubts his intuition.
81+
82+
After chatting with Barbara, who encourages him to benchmark and measure, he decides to switch back to synchronous locks.
83+
84+
Unfortunately, switching back to synchronous locks brings back the old compiler error message about his future not being `Send`. Alan is confused as he's dropping the mutex guard before it ever crosses an await point.
85+
86+
Confused Alan goes to Barbara for advice. She is also confused, and it takes several minutes of exploration before she comes to a solution that works: wrapping the mutex access in a block and implicitly dropping the mutex.
87+
88+
```rust
89+
impl Proxy {
90+
async fn handle(&self, key: String, request: Request) -> crate::Result<Response> {
91+
let route = {
92+
let routes = self.state.lock().unwrap();
93+
routes.get(key).unwrap_or_else(crate::error::MissingRoute)?.clone()
94+
};
95+
Ok(self.client.perform_request(route, request).await?)
96+
}
97+
}
98+
```
99+
100+
Barbara mentions she's unsure why explicitly dropping the mutex guard did not work, but they're both happy that the code compiles. In fact it seems to have improved the performance of the service when its under extreme load. Alan's intuition was right!
101+
102+
In the end, Barbara decides to write a blog post about how blocking in async code isn't always such a bad idea.
103+
104+
## 🤔 Frequently Asked Questions
105+
106+
### **What are the morals of the story?**
107+
* Locks can be quite common in async code as many tasks might need to mutate some shared state.
108+
* Error messages can be fairly good, but they still require a decent understanding of Rust (e.g., `Send`, `MutexGuard`, drop semantics) to fully understand what's going on.
109+
* This can lead to needing to use certain patterns (like dropping mutex guards early) in order to get code working.
110+
* The advice to never block in async code is not always true: if blocking is short enough, is it even blocking at all?
111+
### **What are the sources for this story?**
112+
* Chats with [Alice](https://github.com/Darksonn) and [Lucio](https://github.com/LucioFranco).
113+
* Alice's [blog post](https://ryhl.io/blog/async-what-is-blocking/) on the subject has some good insights.
114+
* The issue of conservative analysis of whether values are used across await points causing futures to be `!Send` is [known](https://rust-lang.github.io/async-book/07_workarounds/03_send_approximation.html), but it takes some digging to find out about this issue. A tracking issue for this can be [found here](https://github.com/rust-lang/rust/issues/57478).
115+
### **Why did you choose [Alan](../characters/alan.md) to tell this story?**
116+
* While Barbara might be tripped up on some of the subtlties, an experienced Rust developer can usually tell how to avoid some of the issues of using locks in async code. Alan on the other hand, might be surprised when his code does not compile as the issue the `Send` error is protecting against (i.e., a mutex guard being moved to another thread) is not protected against in other languages.
117+
### **How would this story have played out differently for the other characters?**
118+
* Grace would have likely had a similar time to Alan. These problems are not necessarily issues you would run into in other languages in the same way.
119+
* Niklaus may have been completely lost. This stuff requires a decent understanding of Rust and of async computational systems.
120+
121+
[character]: ../characters.md
122+
[status quo stories]: ./status_quo.md
123+
[Alan]: ../characters/alan.md
124+
[Grace]: ../characters/grace.md
125+
[Niklaus]: ../characters/niklaus.md
126+
[Barbara]: ../characters/barbara.md
127+
[htvsq]: ../how_to_vision/status_quo.md
128+
[cannot be wrong]: ../how_to_vision/comment.md#comment-to-understand-or-improve-not-to-negate-or-dissuade

src/vision/status_quo/alan_tries_a_socket_sink.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,15 @@ A few weeks later, Alan's work at his project at work is merged with his new for
104104
## 🤔 Frequently Asked Questions
105105

106106

107-
* **What are the morals of the story?**
107+
### **What are the morals of the story?**
108108
* There are often many sources of opinion in the community regarding futures and async, but these opinions aren't always backed up with examples of how it should be better accomplished. Sometimes we just find a thing that works and would prefer to stick with it, but others argue that some traits make implementations unnecessarily complex, and choose to leave it out. Disagreements like these in the ecosystem can be harmful to the reputation of the project and the participants.
109109
* If there's a source of substantial disagreement, the community becomes even further fragmented, and this may cause additional confusion in newcomers.
110110
* Alan is used to fragmentation from the communities he comes from, so this isn't too discouraging, but what's difficult is that there's enough functionality overlap in async libraries that it's tempting to get them to interop with each other as-needed, and this can lead to architectural challenges resulting from a difference in design philosophies.
111111
* It's also unclear if Futures are core to the Rust asynchronous experience, much as Promises are in JavaScript, or if the situation is actually more complex.
112112
* The `Sink` trait is complex but it solves a real problem, and the workarounds required to solve problems without it can be unsatisfactory.
113113
* Disagreement about core abstractions like `Sink` can make interoperability between runtimes more difficult; it also makes it harder for people to reproduce patterns they are used to from one runtime to another.
114114
* It is all too easy for technical discussions like this to become heated; it's important for all participants to try and provide each other with the "benefit of the doubt".
115-
* **What are the sources for this story?**
115+
### **What are the sources for this story?**
116116
* <https://github.com/http-rs/tide-websockets>
117117
* <https://github.com/http-rs/tide-websockets/pull/17> - Third pull request
118118
* <https://github.com/http-rs/tide-websockets/issues/15#issuecomment-797090892> - Suggestion to use a broadcast channel
@@ -122,9 +122,9 @@ A few weeks later, Alan's work at his project at work is merged with his new for
122122
* <https://twitter.com/cryptoquick/status/1370143022801846275>
123123
* <https://twitter.com/cryptoquick/status/1370155726056738817>
124124
* <https://blog.yoshuawuyts.com/rust-streams/#why-we-do-not-talk-about-the-sink-trait>
125-
* **Why did you choose [Alan](../characters/alan.md) to tell this story?**
125+
### **Why did you choose [Alan](../characters/alan.md) to tell this story?**
126126
* Alan is more representative of the original author's background in JS, TypeScript, and NodeJS.
127-
* **How would this story have played out differently for the other characters?**
127+
### **How would this story have played out differently for the other characters?**
128128
* (I'm not sure.)
129129

130130
[character]: ../characters.md

0 commit comments

Comments
 (0)