Skip to content

Commit dd34e31

Browse files
andrewjcgfacebook-github-bot
authored andcommitted
Fix owned read guard bug (#457)
Summary: Pull Request resolved: #457 Make sure `OwnedPreemptibleRwLockReadGuard` holds a reference to the original lock, to prevent it from prematurely getting dropped (just as `OwnedRwLockReadGuard` does). Reviewed By: dcci, mariusae Differential Revision: D77910522 fbshipit-source-id: 11d670dd46f09209b7b24502918fbc0d30aef47d
1 parent 1ff94c2 commit dd34e31

File tree

2 files changed

+27
-1
lines changed

2 files changed

+27
-1
lines changed

preempt_rwlock/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ tokio = { version = "1.45.0", features = ["full", "test-util", "tracing"] }
1212

1313
[dev-dependencies]
1414
anyhow = "1.0.98"
15+
futures = { version = "0.3.30", features = ["async-await", "compat"] }

preempt_rwlock/src/lib.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ impl<T: Sized> std::ops::Deref for PreemptibleRwLockReadGuard<'_, T> {
3838
}
3939

4040
pub struct OwnedPreemptibleRwLockReadGuard<T: ?Sized, U: ?Sized = T> {
41+
lock: Arc<PreemptibleRwLock<T>>,
4142
preemptor: watch::Receiver<usize>,
4243
guard: OwnedRwLockReadGuard<T, U>,
4344
}
@@ -58,6 +59,7 @@ impl<T: ?Sized, U: ?Sized> OwnedPreemptibleRwLockReadGuard<T, U> {
5859
F: FnOnce(&U) -> &V,
5960
{
6061
OwnedPreemptibleRwLockReadGuard {
62+
lock: self.lock,
6163
preemptor: self.preemptor,
6264
guard: OwnedRwLockReadGuard::map(self.guard, f),
6365
}
@@ -106,7 +108,7 @@ impl<T: Sized> std::ops::DerefMut for PreemptibleRwLockWriteGuard<'_, T> {
106108
/// readers get preempted, via `preempted()` method on the read guard that
107109
/// readers can `tokio::select!` on.
108110
#[derive(Debug)]
109-
pub struct PreemptibleRwLock<T: Sized> {
111+
pub struct PreemptibleRwLock<T: ?Sized> {
110112
lock: Arc<RwLock<T>>,
111113
preemptor_lock: RwLock<()>,
112114
// Used to track the number of writers waiting to acquire the lock and
@@ -134,6 +136,7 @@ impl<T: Sized> PreemptibleRwLock<T> {
134136
pub async fn read_owned(self: Arc<Self>) -> OwnedPreemptibleRwLockReadGuard<T> {
135137
let _guard = self.preemptor_lock.read().await;
136138
OwnedPreemptibleRwLockReadGuard {
139+
lock: self.clone(),
137140
preemptor: self.preemptor.1.clone(),
138141
guard: self.lock.clone().read_owned().await,
139142
}
@@ -144,6 +147,7 @@ impl<T: Sized> PreemptibleRwLock<T> {
144147
) -> Result<OwnedPreemptibleRwLockReadGuard<T>, TryLockError> {
145148
let _guard = self.preemptor_lock.try_read()?;
146149
Ok(OwnedPreemptibleRwLockReadGuard {
150+
lock: self.clone(),
147151
preemptor: self.preemptor.1.clone(),
148152
guard: self.lock.clone().try_read_owned()?,
149153
})
@@ -234,4 +238,25 @@ mod tests {
234238

235239
Ok(())
236240
}
241+
242+
#[tokio::test]
243+
#[allow(clippy::disallowed_methods)]
244+
async fn test_preempt_owned_reader_after_lock_dropped() -> Result<()> {
245+
let lock = Arc::new(PreemptibleRwLock::new(42));
246+
247+
// Get an owned read guard
248+
let reader = lock.clone().read_owned().await;
249+
250+
// Drop the original Arc<PreemptibleRwLock>
251+
drop(lock);
252+
253+
// Verify that preempted() still works even after the original Arc is dropped
254+
tokio::select! {
255+
biased; // Make sure peempted is checked first.
256+
_ = reader.preempted() => assert!(false),
257+
_ = futures::future::ready(()) => (),
258+
}
259+
260+
Ok(())
261+
}
237262
}

0 commit comments

Comments
 (0)