RAII chapter for idiomatic rust #2820

GlenDC · 2025-07-15T18:41:43Z

This PR adds the RAII chapter for the idiomatic Rust deep dive.

google-cla · 2025-07-15T18:41:48Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

src/idiomatic/leveraging-the-type-system/raii.md

src/idiomatic/leveraging-the-type-system/raii/drop_limitations.md

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

src/idiomatic/leveraging-the-type-system/raii/scope_guards.md

randomPoison

Looks pretty good, there's lots of good stuff in this chapter 😁 I've got various feedback below, but it's definitely off to a good start!

src/idiomatic/leveraging-the-type-system/raii.md

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

src/idiomatic/leveraging-the-type-system/raii/scope_guards.md

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

src/idiomatic/leveraging-the-type-system/raii/scope_guards.md

src/idiomatic/leveraging-the-type-system/raii.md

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

GlenDC · 2025-07-25T13:16:03Z

I started to action all feedback since today. I'll explicitly re-request reviews once I am done. Probably best not to review again until then.

- directly where possible - otherwise as inline feedback as notes to take into account for next draft

refresher on `RAII` and use the OS File Descriptor example to start the discussions around RAII all previous content is for now moved to `_old` for my own reference, will add the new content based on the agreed upon new structure next.

src/idiomatic/leveraging-the-type-system/raii.md

GlenDC · 2025-08-03T19:34:43Z

The four slides in the RAII chapter are ready for review again.

randomPoison

This is looking pretty good. The drop bomb and scope guard slides look good to me, my biggest suggestion is that we might want to split the mutex slide up so that it flows a bit better.

src/idiomatic/leveraging-the-type-system/raii.md

src/idiomatic/leveraging-the-type-system/raii/mutex.md

djmitche

I had a brief look at the updates since my last comments, and I'm happy enough!

split mutex (RAII) chapter in two parts

GlenDC · 2025-08-30T07:17:59Z

@randomPoison your feedback was all address as far as I know. Ready for another round of reviews.

randomPoison

Thanks for addressing my previous feedback! I've looked over things again and still have some suggestions/requests for how this can be improved. There's a couple of places where I've suggested adding new slides:

Splitting out the discussion of when drop does and doesn't run.
Using mem::forget to defuse drop bombs.
Using Option for cases where you need to transfer ownership in drop.

I'm fine with those being split into a separate PR if you want to get this one landed, though if you do that then it'd be good to open an issue tracking the unfinished changes to this section.

randomPoison · 2025-09-16T20:04:06Z

src/idiomatic/leveraging-the-type-system/raii.md

+- If both `drop()` and `close()` exist, the file descriptor may be released
+  twice. To avoid this, remove `close()` and rely solely on `Drop`.


Having both close and drop is fine, but the close impl needs to mem::forget (or destructure) the File object to prevent drop from also running. That's a normal pattern, especially when close potentially wants to report an error, which can't be done with drop.

I think that's a point worth covering in its own slide. We point out that drop can't return any errors, I think it'd be good to talk about that directly in a slide and demonstrate how you can create a function like close that consumes the object and prevent the normal destructor from running redundantly.

randomPoison · 2025-09-16T21:25:11Z

src/idiomatic/leveraging-the-type-system/raii.md

+- 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
+  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) {}
+  ```
+
+  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:
+  - 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.


I still think this is worth pulling out into its own slide (I previously commented that, idk if it got lost in the shuffle).

randomPoison · 2025-09-16T21:59:41Z

src/idiomatic/leveraging-the-type-system/raii/drop_guards.md

+#[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:?}");
+}


This code example is very verbose, I think it'd be better to simplify this. I think we should remove the atomics and UnsafeCell, and simplify this to a dummy mutex that just locks and unlocks (i.e. doesn't actually wrap any data). I think we can also remove the main function since we already showed what using the mutex looks like on the previous slide. My suggestion would be something like this:

#[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 Mutex is actually implemented.

randomPoison · 2025-09-16T22:09:45Z

src/idiomatic/leveraging-the-type-system/raii/mutex.md

+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.


Suggested change

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.

In earlier examples, RAII was used to manage concrete resources like file

descriptors. With a `Mutex`, the "resource" is mutable access to a value.

You access the value by calling `lock`, which then returns a `MutexGuard`

which will unlock the `Mutex` automatically when dropped.

A couple suggestions for wording here:

I wouldn't say the resource is "abstract", it's just not an external resource the way that a heap allocation or a file handle is.

For this slide I think it'd be better to explain the usage of Mutex so that on the next slide we can dig into how MutexGuard works.

randomPoison · 2025-09-16T22:23:27Z

src/idiomatic/leveraging-the-type-system/raii/drop_guards.md

+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.


Suggested change

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.

A **drop guard** in Rust is a temporary object that performs

some kind of cleanup when it goes out of scope. In the case

of `Mutex`, the `lock` method returns a `MutexGuard` that

automatically unlocks the mutex on drop:

I think more concise wording here would be good to save space for the larger example code on this slide.

randomPoison · 2025-09-16T22:40:17Z

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

+    let tx = Transaction::start();
+
+    if some_condition() {
+        tx.commit()?;
+    } else {
+        tx.rollback()?;
+    }
+
+    // Uncomment to see the panic:
+    // let tx2 = Transaction::start();


Suggested change

let tx = Transaction::start();

if some_condition() {

tx.commit()?;

} else {

tx.rollback()?;

}

// Uncomment to see the panic:

// let tx2 = Transaction::start();

let tx = Transaction::start();

// Use `tx` to build the transaction, then commit it.

// Comment out the call to `commit` to see the panic.

tx.commit()?;

Let's simplify this by removing rollback and tx2. That will help the example fit on the slide better.

randomPoison · 2025-09-16T22:40:55Z

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

+
+fn some_condition() -> bool {
+    // [...]
+    true
+}


This can also be removed once rollback is removed.

randomPoison · 2025-09-16T22:41:46Z

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

+    /// Begin a [`Transaction`].
+    ///
+    /// ## Panics
+    ///
+    /// Panics if the transaction is dropped without
+    /// calling [`Self::commit`] or [`Self::rollback`].


Remove this comment, we don't need documentation in the example code and we're already explaining that this will panic on drop. That will help the code example fit on the slide.

randomPoison · 2025-09-16T22:56:03Z

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

+- [`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
+          }
+      }
+  }
+  ```


I think this deserves its own slide. Having to move out of a field in drop comes up pretty frequently, and this is a common enough pattern that I think it's worth demonstrating explicitly. That can be split out into its own PR if you want.

randomPoison · 2025-09-16T23:08:36Z

src/idiomatic/leveraging-the-type-system/raii/drop_bomb.md

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 drop. This is a reasonable approach to take, but it has the drawback of some additional runtime overhead. There's another way to achieve the same thing without any overhead: use mem::forget in commit to directly prevent drop from running.

I'd like to discuss forget somewhere in this section, since it's very relevant when using Drop, especially in cases where you sometimes want to prevent the regular destructor from running. In another comment I suggested splitting out a slide that discusses when drop is and isn't run, discussing forget there might make sense. Otherwise, it wouldn't hurt to have another slide that demonstrates this transaction example using forget instead of the active flag.

GlenDC requested review from djmitche, gribozavr, randomPoison and LukeMathWalker July 15, 2025 18:41

GlenDC self-assigned this Jul 15, 2025

GlenDC added the deep_dives/idiomatic label Jul 15, 2025

initial version of the raii chapters for idiomatic rust

39ae8f6

GlenDC force-pushed the raii branch from 1c9b479 to 39ae8f6 Compare July 15, 2025 18:46

GlenDC added 2 commits July 15, 2025 20:54

apply dprint fmt

f4269b7

fix build (en) step: scopeguard crate import

bb88a79

gribozavr reviewed Jul 16, 2025

View reviewed changes

randomPoison requested changes Jul 22, 2025

View reviewed changes

djmitche reviewed Jul 22, 2025

View reviewed changes

GlenDC added the waiting for author Waiting on the issue author to reply label Jul 25, 2025

GlenDC added 4 commits July 26, 2025 15:11

integrate feedback of first reviews (RAII)

5a4838e

- directly where possible - otherwise as inline feedback as notes to take into account for next draft

add new RAII intro segment

d804144

refresher on `RAII` and use the OS File Descriptor example to start the discussions around RAII all previous content is for now moved to `_old` for my own reference, will add the new content based on the agreed upon new structure next.

further improvements to RAII intro chapter

5c2ec8c

fix typo

0baa990

LukeMathWalker reviewed Jul 31, 2025

View reviewed changes

src/idiomatic/leveraging-the-type-system/raii.md Outdated Show resolved Hide resolved