-
Notifications
You must be signed in to change notification settings - Fork 1.9k
RAII chapter for idiomatic rust #2820
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
base: main
Are you sure you want to change the base?
Changes from all commits
39ae8f6
f4269b7
bb88a79
5a4838e
d804144
5c2ec8c
0baa990
9fa819a
4ebb43f
42a4237
2923cf3
48c5baa
cc5f3b5
55e4753
1d67c9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,112 @@ | ||||||||||||||||||||||||
--- | ||||||||||||||||||||||||
minutes: 30 | ||||||||||||||||||||||||
--- | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
# RAII: `Drop` trait | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
RAII (**R**esource **A**cquisition **I**s **I**nitialization) ties the lifetime | ||||||||||||||||||||||||
of a resource to the lifetime of a value. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
[Rust uses RAII to manage memory](https://doc.rust-lang.org/rust-by-example/scope/raii.html), | ||||||||||||||||||||||||
and the `Drop` trait allows you to extend this to other resources, such as file | ||||||||||||||||||||||||
descriptors or locks. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
```rust,editable | ||||||||||||||||||||||||
pub struct File(std::os::fd::RawFd); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
impl File { | ||||||||||||||||||||||||
pub fn open(path: &str) -> Result<Self, std::io::Error> { | ||||||||||||||||||||||||
// [...] | ||||||||||||||||||||||||
Ok(Self(0)) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
pub fn read_to_end(&mut self) -> Result<Vec<u8>, std::io::Error> { | ||||||||||||||||||||||||
// [...] | ||||||||||||||||||||||||
Ok(b"example".to_vec()) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
pub fn close(self) -> Result<(), std::io::Error> { | ||||||||||||||||||||||||
// [...] | ||||||||||||||||||||||||
Ok(()) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
fn main() -> Result<(), std::io::Error> { | ||||||||||||||||||||||||
let mut file = File::open("example.txt")?; | ||||||||||||||||||||||||
println!("content: {:?}", file.read_to_end()?); | ||||||||||||||||||||||||
Ok(()) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
``` | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
<details> | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- This example shows how easy it is to forget releasing a file descriptor when | ||||||||||||||||||||||||
managing it manually. The code as written does not call `file.close()`. Did | ||||||||||||||||||||||||
anyone in the class notice? | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- To release the file descriptor correctly, `file.close()` must be called after | ||||||||||||||||||||||||
the last use — and also in early-return paths in case of errors. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- Instead of relying on the user to call `close()`, we can implement the `Drop` | ||||||||||||||||||||||||
trait to release the resource automatically. This ties cleanup to the lifetime | ||||||||||||||||||||||||
of the `File` value. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
```rust,compile_fail | ||||||||||||||||||||||||
impl Drop for File { | ||||||||||||||||||||||||
fn drop(&mut self) { | ||||||||||||||||||||||||
println!("release file descriptor automatically"); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
``` | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- Note that `Drop::drop` cannot return errors. Any fallible logic must be | ||||||||||||||||||||||||
handled internally or ignored. In the standard library, errors returned while | ||||||||||||||||||||||||
closing an owned file descriptor during `Drop` are silently discarded: | ||||||||||||||||||||||||
<https://doc.rust-lang.org/src/std/os/fd/owned.rs.html#169-196> | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- If both `drop()` and `close()` exist, the file descriptor may be released | ||||||||||||||||||||||||
twice. To avoid this, remove `close()` and rely solely on `Drop`. | ||||||||||||||||||||||||
Comment on lines
+67
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having both I think that's a point worth covering in its own slide. We point out that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, agreed. There are cases where it is acceptable define both
GlenDC marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like more explicit instructions for the speaker about how the code should evolve up to this point, and how to demonstrate this. In particular the order in which the instructor should add the drop method, remove the close method, how to demonstrate the double-closing of the file etc. The flow can get very messy during the actual presentation if we don't think this through ahead of time. |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
- When is `Drop::drop` called? | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
Normally, when the `file` variable in `main` goes out of scope (either on | ||||||||||||||||||||||||
return or due to a panic), `drop()` is called automatically. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
If the file is moved into another function, for example `read_all()`, the | ||||||||||||||||||||||||
GlenDC marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
value is dropped when that function returns — not in `main`. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
In contrast, C++ runs destructors in the original scope even for moved-from | ||||||||||||||||||||||||
values. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- The same mechanism powers `std::mem::drop`: | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
```rust | ||||||||||||||||||||||||
pub fn drop<T>(_x: T) {} | ||||||||||||||||||||||||
``` | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The instructor can't speak such a long code example. This needs to be put on a separate slide. |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
You can use it to force early destruction of a value before its natural end of | ||||||||||||||||||||||||
scope. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- Insert `panic!("oops")` at the start of `read_to_end()` to show that `drop()` | ||||||||||||||||||||||||
still runs during unwinding. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- There are cases where destructors will not run: | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be a good idea to pull this bullet point ("There are cases where destructors will not run") into a separate slide. |
||||||||||||||||||||||||
- If a destructor itself panics during unwinding, the program aborts | ||||||||||||||||||||||||
immediately. | ||||||||||||||||||||||||
- If the program exits with `std::process::exit()` or is compiled with the | ||||||||||||||||||||||||
`abort` panic strategy, destructors are skipped. | ||||||||||||||||||||||||
Comment on lines
+70
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think this is worth pulling out into its own slide (I previously commented that, idk if it got lost in the shuffle). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, agreed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is good information, but it is not made actionable for the listener. Quoting my original comment:
I suggest adding an example like that to the speaker notes. Otherwise the information "drop is not run when the program exits" is not connected to any real-world bad outcome / architectural mistake.
Comment on lines
+93
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
### More to Explore | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
The `Drop` trait has another important limitation: it is not `async`. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
This means you cannot `await` inside a destructor, which is often needed when | ||||||||||||||||||||||||
cleaning up asynchronous resources like sockets, database connections, or tasks | ||||||||||||||||||||||||
that must signal completion to another system. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- Learn more: | ||||||||||||||||||||||||
<https://rust-lang.github.io/async-fundamentals-initiative/roadmap/async_drop.html> | ||||||||||||||||||||||||
- There is an experimental `AsyncDrop` trait available on nightly: | ||||||||||||||||||||||||
<https://doc.rust-lang.org/nightly/std/future/trait.AsyncDrop.html> | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
</details> | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Summary: I like the ideas behind this slide, there is a lot of good content, and it illustrates many important points about type design in Rust.
|
GlenDC marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The example on this slide demonstrates using a runtime flag to track if the transaction has been finalized, and then checks that flag to conditionally panic in I'd like to discuss |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,123 @@ | ||||||||||||||||||||||||||||||
# Drop Bombs: Enforcing API Correctness | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
Use `Drop` to enforce invariants and detect incorrect API usage. A "drop bomb" | ||||||||||||||||||||||||||||||
panics if a value is dropped without being explicitly finalized. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
This pattern is often used when the finalizing operation (like `commit()` or | ||||||||||||||||||||||||||||||
`rollback()`) needs to return a `Result`, which cannot be done from `Drop`. | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before hitting the audience with a long code example, maybe show them a diagram that illustrates a good and a bad method call sequence? (abstractly, just method names, without any code) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not opposed to adding a diagram, but I think the better solution would be to simplify the example enough that we don't need a diagram to explain it. I've already left a handful of notes suggesting changes, with those applied I think the example code would be sufficiently short. |
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
```rust,editable | ||||||||||||||||||||||||||||||
use std::io::{self, Write}; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
struct Transaction { | ||||||||||||||||||||||||||||||
active: bool, | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
impl Transaction { | ||||||||||||||||||||||||||||||
/// Begin a [`Transaction`]. | ||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||
/// ## Panics | ||||||||||||||||||||||||||||||
/// | ||||||||||||||||||||||||||||||
/// Panics if the transaction is dropped without | ||||||||||||||||||||||||||||||
/// calling [`Self::commit`] or [`Self::rollback`]. | ||||||||||||||||||||||||||||||
GlenDC marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||
fn start() -> Self { | ||||||||||||||||||||||||||||||
Self { active: true } | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
fn commit(mut self) -> io::Result<()> { | ||||||||||||||||||||||||||||||
writeln!(io::stdout(), "COMMIT")?; | ||||||||||||||||||||||||||||||
self.active = false; | ||||||||||||||||||||||||||||||
Ok(()) | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
fn rollback(mut self) -> io::Result<()> { | ||||||||||||||||||||||||||||||
writeln!(io::stdout(), "ROLLBACK")?; | ||||||||||||||||||||||||||||||
self.active = false; | ||||||||||||||||||||||||||||||
Ok(()) | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
Comment on lines
+33
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1. Alternatively, simplify the body to just a |
||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
impl Drop for Transaction { | ||||||||||||||||||||||||||||||
fn drop(&mut self) { | ||||||||||||||||||||||||||||||
if self.active { | ||||||||||||||||||||||||||||||
panic!("Transaction dropped without commit or rollback!"); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
fn main() -> io::Result<()> { | ||||||||||||||||||||||||||||||
let tx = Transaction::start(); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if some_condition() { | ||||||||||||||||||||||||||||||
tx.commit()?; | ||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||
tx.rollback()?; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// Uncomment to see the panic: | ||||||||||||||||||||||||||||||
// let tx2 = Transaction::start(); | ||||||||||||||||||||||||||||||
Comment on lines
+49
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Let's simplify this by removing |
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
Ok(()) | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
fn some_condition() -> bool { | ||||||||||||||||||||||||||||||
// [...] | ||||||||||||||||||||||||||||||
true | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
Comment on lines
+62
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can also be removed once |
||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
<details> | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- This pattern ensures that a value like `Transaction` cannot be silently | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||
dropped in an unfinished state. The destructor panics if neither `commit()` | ||||||||||||||||||||||||||||||
nor `rollback()` has been called. | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- A common reason to use this pattern is when cleanup cannot be done in `Drop`, | ||||||||||||||||||||||||||||||
either because it is fallible or asynchronous. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- This pattern is appropriate even in public APIs. It can help users catch bugs | ||||||||||||||||||||||||||||||
early when they forget to explicitly finalize a transactional object. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- If a value can be safely cleaned up in `Drop`, consider falling back to that | ||||||||||||||||||||||||||||||
behavior in Release mode and panicking only in Debug. This decision should be | ||||||||||||||||||||||||||||||
made based on the guarantees your API provides. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- Panicking in Release builds is a valid choice if silent misuse could lead to | ||||||||||||||||||||||||||||||
serious correctness issues or security concerns. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
## Additional Patterns | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- [`Option<T>` with `.take()`](https://doc.rust-lang.org/std/option/enum.Option.html#method.take): | ||||||||||||||||||||||||||||||
A common pattern inside `Drop` to move out internal values and prevent double | ||||||||||||||||||||||||||||||
drops. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
```rust,compile_fail | ||||||||||||||||||||||||||||||
impl Drop for MyResource { | ||||||||||||||||||||||||||||||
fn drop(&mut self) { | ||||||||||||||||||||||||||||||
if let Some(handle) = self.handle.take() { | ||||||||||||||||||||||||||||||
// do cleanup with handle | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||
Comment on lines
+90
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this deserves its own slide. Having to move out of a field in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, agreed. |
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- [`ManuallyDrop`](https://doc.rust-lang.org/std/mem/struct.ManuallyDrop.html): | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this and below is "more to explore" content. |
||||||||||||||||||||||||||||||
Prevents automatic destruction and gives full manual control. Requires | ||||||||||||||||||||||||||||||
`unsafe`, so only use when strictly necessary. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- [`drop_bomb` crate](https://docs.rs/drop_bomb/latest/drop_bomb/): A small | ||||||||||||||||||||||||||||||
utility that panics if dropped unless explicitly defused with `.defuse()`. | ||||||||||||||||||||||||||||||
Comes with a `DebugDropBomb` variant that only activates in debug builds. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
- In some systems, a value must be finalized by a specific API before it is | ||||||||||||||||||||||||||||||
dropped. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
For example, an `SshConnection` might need to be deregistered from an | ||||||||||||||||||||||||||||||
`SshServer` before being dropped, or the program panics. This helps catch | ||||||||||||||||||||||||||||||
programming mistakes during development and enforces correct teardown at | ||||||||||||||||||||||||||||||
runtime. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
See a working example in | ||||||||||||||||||||||||||||||
[the Rust playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=3223f5fa5e821cd32461c3af7162cd55). | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please inline the example here. We should prefer to have content we own in the repo, rather than behind links we don't control. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it might be better to remove this note entirely. The linked code is demonstrating the same thing that the example code in the slide does, I don't think it adds much to the discussion, and I wouldn't want the linked code inlined in the speaker notes as that would add a lot of noise. |
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
</details> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,85 @@ | ||||||||||||||||||||||||||
# Drop Guards | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
A **drop guard** in Rust is a temporary _RAII_ guard that executes a specific | ||||||||||||||||||||||||||
action when it goes out of scope. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
It acts as a wrapper around a value, ensuring that some cleanup or secondary | ||||||||||||||||||||||||||
behavior happens automatically when the guard is dropped. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
One of the most common examples is `MutexGuard`, which represents temporary | ||||||||||||||||||||||||||
exclusive access to a shared resource. | ||||||||||||||||||||||||||
Comment on lines
+3
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think more concise wording here would be good to save space for the larger example code on this slide. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, agreed. You can move the longer version to a bullet in the speaker notes. |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
```rust | ||||||||||||||||||||||||||
#[derive(Debug)] | ||||||||||||||||||||||||||
struct Mutex<T> { | ||||||||||||||||||||||||||
value: std::cell::UnsafeCell<T>, | ||||||||||||||||||||||||||
is_locked: std::sync::atomic::AtomicBool, | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
#[derive(Debug)] | ||||||||||||||||||||||||||
struct MutexGuard<'a, T> { | ||||||||||||||||||||||||||
value: &'a mut T, | ||||||||||||||||||||||||||
mutex: &'a Mutex<T>, | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
impl<T> Mutex<T> { | ||||||||||||||||||||||||||
fn new(value: T) -> Self { | ||||||||||||||||||||||||||
Self { | ||||||||||||||||||||||||||
value: std::cell::UnsafeCell::new(value), | ||||||||||||||||||||||||||
is_locked: std::sync::atomic::AtomicBool::new(false), | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
fn lock(&self) -> MutexGuard<'_, T> { | ||||||||||||||||||||||||||
// Acquire the lock and create the guard object. | ||||||||||||||||||||||||||
if self.is_locked.swap(true, std::sync::atomic::Ordering::AcqRel) { | ||||||||||||||||||||||||||
todo!("Block until the lock is released"); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
let value = unsafe { &mut *self.value.get() }; | ||||||||||||||||||||||||||
MutexGuard { value, mutex: self } | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
impl<'a, T> Drop for MutexGuard<'a, T> { | ||||||||||||||||||||||||||
fn drop(&mut self) { | ||||||||||||||||||||||||||
self.mutex.is_locked.store(false, std::sync::atomic::Ordering::Release); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
fn main() { | ||||||||||||||||||||||||||
let m = Mutex::new(vec![1, 2, 3]); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
let mut guard = m.lock(); | ||||||||||||||||||||||||||
guard.value.push(4); | ||||||||||||||||||||||||||
guard.value.push(5); | ||||||||||||||||||||||||||
println!("{guard:?}"); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+13
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code example is very verbose, I think it'd be better to simplify this. I think we should remove the atomics and #[derive(Debug)]
struct Mutex {
is_locked: bool,
}
#[derive(Debug)]
struct MutexGuard<'a> {
mutex: &'a mut Mutex<T>,
}
impl Mutex {
fn new() -> Self {
Self {
is_locked: false,
}
}
fn lock(&mut self) -> MutexGuard<'_> {
self.is_locked = true;
MutexGuard { mutex: self }
}
}
impl<'a> Drop for MutexGuard<'a> {
fn drop(&mut self) {
self.mutex.is_locked = false;
}
} This is obviously not a realistic mutex, but it's far clearer from the perspective of illustrating the drop guard pattern to students. I think it'd be enough to have a speaker note that points out that this is not accurate to how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I definitely agree that the example is too long for a single slide and for the point we're trying to illustrate (drop guards). I like your suggested simplified code. I would like to suggest a few things in addition:
Something like this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's fine to have a note that this is not a realistic example, but I'd suggest keeping it concise to avoid distracting from the actual point of the slide (i.e demonstrating the guard pattern). I'd prefer not to have a TODO comment in the example code, that just adds noise and isn't really relevant since this is not trying to be an accurate mutex implementation. I also don't think it's necessary to say that this is like a C++ mutex, the distinction between a mutex that wraps data vs a mutex that protects a critical section is entirely tangential to the guard object pattern. If you want to describe how this example differs from a real mutex, or make a comparison to C++ mutexes, then I'd prefer that those go into a "More to Explore" section at the end of the notes to avoid cluttering the speaker notes section. |
||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
<details> | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
- The example above shows a simplified `Mutex` and its associated guard. Even | ||||||||||||||||||||||||||
though it is not a production-ready implementation, it illustrates the core | ||||||||||||||||||||||||||
idea: the guard enforces exclusive access, and its `Drop` implementation | ||||||||||||||||||||||||||
guarantees that the lock is released when the guard goes out of scope. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
- A few things are left out for brevity: | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
- `Deref` and `DerefMut` implementations for `MutexGuard`, which would allow | ||||||||||||||||||||||||||
you to use the guard as if it were a direct reference to the inner value. | ||||||||||||||||||||||||||
- Making `.lock()` truly blocking, so that it waits until the mutex is free | ||||||||||||||||||||||||||
before returning. | ||||||||||||||||||||||||||
- In addition, a `.try_lock()` method could be added to provide a | ||||||||||||||||||||||||||
non-blocking alternative, returning `Option::None` or `Result::Err(...)` | ||||||||||||||||||||||||||
if the mutex is still locked. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
- Panics are not explicitly handled in the `Drop` implementation here. In | ||||||||||||||||||||||||||
practice, one can use `std::thread::panicking()` to check if the guard was | ||||||||||||||||||||||||||
dropped during a panic. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
- The standard library’s `std::sync::Mutex` uses this to implement | ||||||||||||||||||||||||||
**poisoning**, where a mutex is marked as poisoned if a panic occurs while | ||||||||||||||||||||||||||
holding the lock, since the protected value may now be in an inconsistent | ||||||||||||||||||||||||||
Comment on lines
+66
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove these points about why the example |
||||||||||||||||||||||||||
state. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
</details> |
GlenDC marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,54 @@ | ||||||||||||||||||||||
# Mutex and MutexGuard | ||||||||||||||||||||||
|
||||||||||||||||||||||
In earlier examples, RAII was used to manage concrete resources like file | ||||||||||||||||||||||
descriptors. With a `Mutex`, the resource is more abstract: exclusive access to | ||||||||||||||||||||||
a value. | ||||||||||||||||||||||
|
||||||||||||||||||||||
Rust models this using a `MutexGuard`, which ties access to a critical section | ||||||||||||||||||||||
to the lifetime of a value on the stack. | ||||||||||||||||||||||
Comment on lines
+3
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
A couple suggestions for wording here:
|
||||||||||||||||||||||
|
||||||||||||||||||||||
```rust | ||||||||||||||||||||||
use std::sync::Mutex; | ||||||||||||||||||||||
|
||||||||||||||||||||||
fn main() { | ||||||||||||||||||||||
let m = Mutex::new(vec![1, 2, 3]); | ||||||||||||||||||||||
|
||||||||||||||||||||||
let mut guard = m.lock().unwrap(); | ||||||||||||||||||||||
guard.push(4); | ||||||||||||||||||||||
guard.push(5); | ||||||||||||||||||||||
println!("{guard:?}"); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
``` | ||||||||||||||||||||||
|
||||||||||||||||||||||
<details> | ||||||||||||||||||||||
|
||||||||||||||||||||||
- A `Mutex` controls exclusive access to a value. Unlike earlier RAII examples, | ||||||||||||||||||||||
the resource here is not external but logical: the right to mutate shared | ||||||||||||||||||||||
data. | ||||||||||||||||||||||
|
||||||||||||||||||||||
- This right is represented by a `MutexGuard`. Only one can exist at a time. | ||||||||||||||||||||||
While it lives, it provides `&mut T` access — enforced using `UnsafeCell`. | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not fully sure what "enforced" means here. UnsafeCell is unsafe because it does not enforce all of its invariants itself. There are two ways to handle this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think mentioning |
||||||||||||||||||||||
|
||||||||||||||||||||||
- Although `lock()` takes `&self`, it returns a `MutexGuard` with mutable | ||||||||||||||||||||||
access. This is possible through interior mutability: a common pattern for | ||||||||||||||||||||||
safe shared-state mutation. | ||||||||||||||||||||||
|
||||||||||||||||||||||
- `MutexGuard` implements `Deref` and `DerefMut`, making access ergonomic. You | ||||||||||||||||||||||
lock the mutex, use the guard like a `&mut T`, and the lock is released | ||||||||||||||||||||||
automatically when the guard goes out of scope. | ||||||||||||||||||||||
|
||||||||||||||||||||||
- The release is handled by `Drop`. There is no need to call a separate unlock | ||||||||||||||||||||||
function — this is RAII in action. | ||||||||||||||||||||||
|
||||||||||||||||||||||
## Poisoning | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend splitting this into its own slide:
Poisoning is tangential to the main topic of this section (RAII) so be sure to mark the second slide as skippable if there's time pressure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should discuss mutex poisoning. As you said, it's tangential to the point of this section, and we don't need to discuss poisoning to demonstrate the guard object pattern. The only reason to mention poisoning is to explain why we do |
||||||||||||||||||||||
|
||||||||||||||||||||||
- If a thread panics while holding the lock, the value may be in a corrupt | ||||||||||||||||||||||
state. | ||||||||||||||||||||||
|
||||||||||||||||||||||
- To signal this, the standard library uses poisoning. When `Drop` runs during a | ||||||||||||||||||||||
panic, the mutex marks itself as poisoned. | ||||||||||||||||||||||
|
||||||||||||||||||||||
- On the next `lock()`, this shows up as an error. The caller must decide | ||||||||||||||||||||||
whether to proceed or handle the error differently. | ||||||||||||||||||||||
|
||||||||||||||||||||||
</details> |
Uh oh!
There was an error while loading. Please reload this page.