From cf6a23fefc7d8a3091f3303360a46e15b46ac9c1 Mon Sep 17 00:00:00 2001 From: Orthur Date: Sun, 11 Oct 2026 11:52:36 -0400 Subject: [PATCH 1/2] fix(rwlock): remove downgrade from mapped write guards --- CHANGELOG.md | 4 ++ asyncband/src/rwlock/mapped_write_guard.rs | 14 +++--- asyncband/src/rwlock/mod.rs | 29 +++++++++++- .../src/rwlock/owned_mapped_write_guard.rs | 14 +++--- asyncband/src/rwlock/owned_write_guard.rs | 3 ++ asyncband/src/rwlock/write_guard.rs | 3 ++ tests-integration/tests/rwlock_test.rs | 18 ++++---- tests-integration/tests/unsafe_paths_test.rs | 45 ++++++++++++++++++- 8 files changed, 99 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd5e84c9..fe4c0458 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Breaking changes + +* Remove `MappedRwLockWriteGuard::downgrade` and `OwnedMappedRwLockWriteGuard::downgrade`, which could cause undefined behavior when the mapped component is reachable by other readers through interior mutability. To keep shared access to a component, downgrade the original write guard first and then map it with `RwLockReadGuard::map` or `OwnedRwLockReadGuard::map`. + ### New features * Add bounded MPMC `reserve` and `try_reserve` methods returning a borrowed `Permit`, so callers can wait for capacity before constructing a value; sends and reservations receive capacity in wait-queue order, and unused permits release it. diff --git a/asyncband/src/rwlock/mapped_write_guard.rs b/asyncband/src/rwlock/mapped_write_guard.rs index 8c2f9b4b..20868de6 100644 --- a/asyncband/src/rwlock/mapped_write_guard.rs +++ b/asyncband/src/rwlock/mapped_write_guard.rs @@ -29,13 +29,17 @@ use std::ops::DerefMut; use std::ptr::NonNull; use crate::internal::semaphore::Semaphore; -use crate::rwlock::MappedRwLockReadGuard; use crate::rwlock::access::WriteAccess; /// Exclusive access to a projection of a locked value borrowed for the guard lifetime. /// /// Use [`RwLockWriteGuard::map`](crate::rwlock::RwLockWriteGuard::map) to select a component. /// Dropping the guard releases its access. +/// +/// A mapped write guard cannot be downgraded, because its projection may reach data that other +/// readers could change through interior mutability. To keep shared access to a component, +/// downgrade the original write guard first and then use +/// [`RwLockReadGuard::map`](crate::rwlock::RwLockReadGuard::map). #[must_use = "dropping the guard releases its write access immediately"] pub struct MappedRwLockWriteGuard<'a, T: ?Sized> { d: NonNull, @@ -123,12 +127,4 @@ impl<'a, T: ?Sized> MappedRwLockWriteGuard<'a, T> { None => Err(orig), } } - - /// Retains shared access to the same projection while releasing exclusive access. - /// - /// There is no unlocked interval in which another writer can modify the value. Queued - /// requests retain their order, so a waiting writer can prevent later readers from joining. - pub fn downgrade(self) -> MappedRwLockReadGuard<'a, T> { - MappedRwLockReadGuard::new(self.d, self.access.downgrade()) - } } diff --git a/asyncband/src/rwlock/mod.rs b/asyncband/src/rwlock/mod.rs index a806d036..39b4cdf2 100644 --- a/asyncband/src/rwlock/mod.rs +++ b/asyncband/src/rwlock/mod.rs @@ -60,6 +60,10 @@ //! can be repeated, and `filter_map` returns the original guard when the component is absent. //! A projection closure that panics releases its guard during unwinding. //! +//! Mapped write guards cannot be downgraded. To keep shared access to a component after an +//! update, downgrade the write guard first and then map the read guard, as in +//! `RwLockReadGuard::map(guard.downgrade(), |items| &items[0])`. +//! //! ``` //! use asyncband::rwlock::MappedRwLockWriteGuard; //! use asyncband::rwlock::RwLock; @@ -71,7 +75,6 @@ //! let slot = RwLockWriteGuard::map(queue.write().await, |items| &mut items[0]); //! let mut message = MappedRwLockWriteGuard::filter_map(slot, Option::as_mut).unwrap(); //! message.push_str(" review"); -//! let message = message.downgrade(); //! assert_eq!(&*message, "pending review"); //! # } //! ``` @@ -250,4 +253,28 @@ mod compile_fail_tests { /// } /// ``` struct OwnedMappedRwLockWriteGuardIsInvariant; + + /// ```compile_fail + /// use asyncband::rwlock::RwLock; + /// use asyncband::rwlock::RwLockWriteGuard; + /// + /// let lock = RwLock::new((0, 0)); + /// let guard = lock.try_write().unwrap(); + /// let guard = RwLockWriteGuard::map(guard, |value| &mut value.0); + /// let _ = guard.downgrade(); + /// ``` + struct MappedRwLockWriteGuardCannotDowngrade; + + /// ```compile_fail + /// use std::sync::Arc; + /// + /// use asyncband::rwlock::OwnedRwLockWriteGuard; + /// use asyncband::rwlock::RwLock; + /// + /// let lock = Arc::new(RwLock::new((0, 0))); + /// let guard = lock.try_write_owned().unwrap(); + /// let guard = OwnedRwLockWriteGuard::map(guard, |value| &mut value.0); + /// let _ = guard.downgrade(); + /// ``` + struct OwnedMappedRwLockWriteGuardCannotDowngrade; } diff --git a/asyncband/src/rwlock/owned_mapped_write_guard.rs b/asyncband/src/rwlock/owned_mapped_write_guard.rs index d1b349c3..359c57cf 100644 --- a/asyncband/src/rwlock/owned_mapped_write_guard.rs +++ b/asyncband/src/rwlock/owned_mapped_write_guard.rs @@ -29,7 +29,6 @@ use std::ops::DerefMut; use std::ptr::NonNull; use std::sync::Arc; -use crate::rwlock::OwnedMappedRwLockReadGuard; use crate::rwlock::RwLock; use crate::rwlock::access::WriteAccess; @@ -37,6 +36,11 @@ use crate::rwlock::access::WriteAccess; /// /// Use [`OwnedRwLockWriteGuard::map`](crate::rwlock::OwnedRwLockWriteGuard::map) to select a /// component. Dropping the guard releases its access. +/// +/// A mapped write guard cannot be downgraded, because its projection may reach data that other +/// readers could change through interior mutability. To keep shared access to a component, +/// downgrade the original write guard first and then use +/// [`OwnedRwLockReadGuard::map`](crate::rwlock::OwnedRwLockReadGuard::map). #[must_use = "dropping the guard releases its write access immediately"] pub struct OwnedMappedRwLockWriteGuard { d: NonNull, @@ -125,12 +129,4 @@ impl OwnedMappedRwLockWriteGuard { None => Err(orig), } } - - /// Retains shared access to the same projection while releasing exclusive access. - /// - /// There is no unlocked interval in which another writer can modify the value. Queued - /// requests retain their order, so a waiting writer can prevent later readers from joining. - pub fn downgrade(self) -> OwnedMappedRwLockReadGuard { - OwnedMappedRwLockReadGuard::new(self.d, self.access.downgrade()) - } } diff --git a/asyncband/src/rwlock/owned_write_guard.rs b/asyncband/src/rwlock/owned_write_guard.rs index f97b2bca..0838589e 100644 --- a/asyncband/src/rwlock/owned_write_guard.rs +++ b/asyncband/src/rwlock/owned_write_guard.rs @@ -138,6 +138,9 @@ impl OwnedRwLockWriteGuard { /// /// There is no unlocked interval in which another writer can modify the value. Queued /// requests retain their order, so a waiting writer can prevent later readers from joining. + /// + /// To keep shared access to a component, call this before [`OwnedRwLockReadGuard::map`] or + /// [`OwnedRwLockReadGuard::filter_map`]. Mapped write guards cannot be downgraded. pub fn downgrade(self) -> OwnedRwLockReadGuard { OwnedRwLockReadGuard { access: self.access.downgrade(), diff --git a/asyncband/src/rwlock/write_guard.rs b/asyncband/src/rwlock/write_guard.rs index 965adfbd..4ba6065e 100644 --- a/asyncband/src/rwlock/write_guard.rs +++ b/asyncband/src/rwlock/write_guard.rs @@ -133,6 +133,9 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> { /// /// There is no unlocked interval in which another writer can modify the value. Queued /// requests retain their order, so a waiting writer can prevent later readers from joining. + /// + /// To keep shared access to a component, call this before [`RwLockReadGuard::map`] or + /// [`RwLockReadGuard::filter_map`]. Mapped write guards cannot be downgraded. pub fn downgrade(self) -> RwLockReadGuard<'a, T> { RwLockReadGuard { access: self.access.downgrade(), diff --git a/tests-integration/tests/rwlock_test.rs b/tests-integration/tests/rwlock_test.rs index ca30f49c..ba6ed897 100644 --- a/tests-integration/tests/rwlock_test.rs +++ b/tests-integration/tests/rwlock_test.rs @@ -143,7 +143,6 @@ async fn owned_write_mappings_preserve_lock_ownership() { let mut value = OwnedMappedRwLockWriteGuard::filter_map(value, |value| value.first_mut()).unwrap(); *value = 100; - let value = value.downgrade(); drop(rwlock); assert!(weak.upgrade().is_some()); @@ -183,7 +182,7 @@ async fn owned_read_mappings_preserve_lock_ownership() { } #[tokio::test] -async fn every_write_guard_type_can_downgrade() { +async fn write_guards_downgrade_before_mapping() { let rwlock = Arc::new(RwLock::new((42, "test".to_string()))); { @@ -212,23 +211,22 @@ async fn every_write_guard_type_can_downgrade() { } { - let write_guard = rwlock.write().await; - let mut mapped_write = RwLockWriteGuard::map(write_guard, |data| &mut data.0); - *mapped_write = 200; + let mut write_guard = rwlock.write().await; + write_guard.0 = 200; - let mapped_read = mapped_write.downgrade(); + let mapped_read = RwLockReadGuard::map(write_guard.downgrade(), |data| &data.0); assert_eq!(*mapped_read, 200); assert!(rwlock.try_write().is_none()); + assert_eq!(rwlock.try_read().unwrap().0, 200); drop(mapped_read); } { - let owned_write = rwlock.clone().write_owned().await; - let mut owned_mapped = OwnedRwLockWriteGuard::map(owned_write, |data| &mut data.1); - *owned_mapped = "final".to_string(); + let mut owned_write = rwlock.clone().write_owned().await; + owned_write.1 = "final".to_string(); - let owned_mapped_read = owned_mapped.downgrade(); + let owned_mapped_read = OwnedRwLockReadGuard::map(owned_write.downgrade(), |data| &data.1); assert_eq!(*owned_mapped_read, "final"); let moved_task = tokio::spawn(async move { diff --git a/tests-integration/tests/unsafe_paths_test.rs b/tests-integration/tests/unsafe_paths_test.rs index 09176281..d19b266f 100644 --- a/tests-integration/tests/unsafe_paths_test.rs +++ b/tests-integration/tests/unsafe_paths_test.rs @@ -21,6 +21,7 @@ use std::marker::PhantomPinned; use std::pin::Pin; use std::pin::pin; use std::ptr; +use std::sync; use std::sync::Arc; use std::task::Context; use std::task::Poll; @@ -91,8 +92,8 @@ fn mapped_rwlock_guards_preserve_lock_ownership() { let mapped = RwLockWriteGuard::map(write, |data| &mut data.values); let mut mapped = MappedRwLockWriteGuard::map(mapped, |values| &mut values[1]); *mapped = 3; - let mapped = mapped.downgrade(); assert_eq!(*mapped, 3); + assert!(lock.try_read().is_none()); assert!(lock.try_write().is_none()); drop(mapped); @@ -113,8 +114,8 @@ fn mapped_rwlock_guards_preserve_lock_ownership() { let mut mapped = OwnedMappedRwLockWriteGuard::filter_map(mapped, |values| values.first_mut()).unwrap(); *mapped = 6; - let mapped = mapped.downgrade(); assert_eq!(*mapped, 6); + assert!(lock.try_read().is_none()); drop(mapped); let read = lock.clone().try_read_owned().unwrap(); @@ -128,6 +129,46 @@ fn mapped_rwlock_guards_preserve_lock_ownership() { assert!(weak.upgrade().is_none()); } +#[test] +fn downgraded_shared_projection_preserves_interior_mutability() { + let lock = RwLock::new(Some(sync::Mutex::new(0u8))); + let mut write = lock.try_write().unwrap(); + *write.as_mut().unwrap().get_mut().unwrap() = 1; + let read = RwLockReadGuard::map(write.downgrade(), |value| value.as_ref().unwrap()); + + // The shared projection retains the mutex, so access to its contents remains synchronized. + let shared = &*read; + let outer = lock.try_read().unwrap(); + *outer.as_ref().unwrap().lock().unwrap() = 2; + assert_eq!(*shared.lock().unwrap(), 2); + assert!(lock.try_write().is_none()); + + drop(outer); + drop(read); + assert!(lock.try_write().is_some()); +} + +#[test] +fn owned_downgraded_shared_projection_preserves_interior_mutability() { + let lock = Arc::new(RwLock::new(Some(sync::Mutex::new(0u8)))); + let weak = Arc::downgrade(&lock); + let mut write = lock.clone().try_write_owned().unwrap(); + *write.as_mut().unwrap().get_mut().unwrap() = 1; + let read = OwnedRwLockReadGuard::filter_map(write.downgrade(), Option::as_ref).unwrap(); + + let shared = &*read; + let outer = lock.try_read().unwrap(); + *outer.as_ref().unwrap().lock().unwrap() = 2; + assert_eq!(*shared.lock().unwrap(), 2); + assert!(lock.try_write().is_none()); + + drop(outer); + drop(lock); + assert_eq!(*shared.lock().unwrap(), 2); + drop(read); + assert!(weak.upgrade().is_none()); +} + struct AddressSensitiveFuture { address: Cell<*const Self>, _pin: PhantomPinned, From ed498e56e21bb121eec1d9337c6814049fd34404 Mon Sep 17 00:00:00 2001 From: Orthur Date: Mon, 12 Oct 2026 06:10:17 -0400 Subject: [PATCH 2/2] docs(rwlock): state where mapped read guard pointers come from --- asyncband/src/rwlock/mapped_read_guard.rs | 7 ++++++- asyncband/src/rwlock/owned_mapped_read_guard.rs | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/asyncband/src/rwlock/mapped_read_guard.rs b/asyncband/src/rwlock/mapped_read_guard.rs index 197d6461..6d88c1b9 100644 --- a/asyncband/src/rwlock/mapped_read_guard.rs +++ b/asyncband/src/rwlock/mapped_read_guard.rs @@ -54,6 +54,9 @@ unsafe impl Send for MappedRwLockReadGuard<'_, T> {} unsafe impl Sync for MappedRwLockReadGuard<'_, T> {} impl<'a, T: ?Sized> MappedRwLockReadGuard<'a, T> { + /// `d` must come from a shared reference into the locked value, created while `access` was + /// held. A pointer derived from `&mut` access is not valid here, because other readers can + /// change its target through interior mutability. pub(crate) fn new(d: NonNull, access: ReadAccess<&'a Semaphore>) -> Self { Self { d, @@ -78,7 +81,9 @@ impl fmt::Display for MappedRwLockReadGuard<'_, T> { impl Deref for MappedRwLockReadGuard<'_, T> { type Target = T; fn deref(&self) -> &Self::Target { - // SAFETY: we hold the read lock and the NonNull pointer is valid for the guard's lifetime + // SAFETY: `access` holds a read permit for the guard's lifetime, so no writer can reach the + // value. `d` comes from a shared reference into the locked value (see `new`), so its target + // changes only inside an `UnsafeCell`, as a shared reference permits. unsafe { self.d.as_ref() } } } diff --git a/asyncband/src/rwlock/owned_mapped_read_guard.rs b/asyncband/src/rwlock/owned_mapped_read_guard.rs index 512d6523..11bd0964 100644 --- a/asyncband/src/rwlock/owned_mapped_read_guard.rs +++ b/asyncband/src/rwlock/owned_mapped_read_guard.rs @@ -53,6 +53,9 @@ unsafe impl Send for OwnedMappedRwLoc unsafe impl Sync for OwnedMappedRwLockReadGuard {} impl OwnedMappedRwLockReadGuard { + /// `d` must come from a shared reference into the locked value, created while `access` was + /// held. A pointer derived from `&mut` access is not valid here, because other readers can + /// change its target through interior mutability. pub(crate) fn new(d: NonNull, access: ReadAccess>>) -> Self { Self { d, @@ -77,7 +80,9 @@ impl fmt::Display for OwnedMappedRwLockRead impl Deref for OwnedMappedRwLockReadGuard { type Target = U; fn deref(&self) -> &Self::Target { - // SAFETY: we hold the read lock and the NonNull pointer is valid for the guard's lifetime + // SAFETY: `access` keeps the lock alive and holds a read permit, so no writer can reach the + // value. `d` comes from a shared reference into the locked value (see `new`), so its target + // changes only inside an `UnsafeCell`, as a shared reference permits. unsafe { self.d.as_ref() } } }