Skip to content

Commit

Permalink
sync: add mem::forget to RwLockWriteGuard::downgrade. (#2957)
Browse files Browse the repository at this point in the history
Currently when `RwLockWriteGuard::downgrade` the `MAX_READS - 1`
permits are added to the semaphore. When `RwLockWriteGuard::drop`
gets invoked however another `MAX_READS` permits are added. This
results in releasing more permits that were actually aquired when
downgrading a write to a read lock. This is why we need to call
`mem::forget` on the `RwLockWriteGuard` in order to avoid
invoking the destructor.

Fixes: #2941
  • Loading branch information
zaharidichev committed Oct 23, 2020
1 parent c153913 commit e804f88
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
5 changes: 2 additions & 3 deletions tokio/src/sync/rwlock.rs
Expand Up @@ -375,8 +375,6 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
/// let n = n.downgrade();
/// assert_eq!(*n, 1, "downgrade is atomic");
///
/// assert_eq!(*lock.read().await, 1, "additional readers can obtain locks");
///
/// drop(n);
/// handle.await.unwrap();
/// assert_eq!(*lock.read().await, 2, "second writer obtained write lock");
Expand All @@ -389,7 +387,8 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {

// Release all but one of the permits held by the write guard
s.release(MAX_READS - 1);

// NB: Forget to avoid drop impl from being called.
mem::forget(self);
RwLockReadGuard {
s,
data,
Expand Down
25 changes: 23 additions & 2 deletions tokio/src/sync/tests/loom_rwlock.rs
Expand Up @@ -6,7 +6,7 @@ use std::sync::Arc;

#[test]
fn concurrent_write() {
let mut b = loom::model::Builder::new();
let b = loom::model::Builder::new();

b.check(|| {
let rwlock = Arc::new(RwLock::<u32>::new(0));
Expand Down Expand Up @@ -37,7 +37,7 @@ fn concurrent_write() {

#[test]
fn concurrent_read_write() {
let mut b = loom::model::Builder::new();
let b = loom::model::Builder::new();

b.check(|| {
let rwlock = Arc::new(RwLock::<u32>::new(0));
Expand Down Expand Up @@ -76,3 +76,24 @@ fn concurrent_read_write() {
assert_eq!(10, *guard);
});
}
#[test]
fn downgrade() {
loom::model(|| {
let lock = Arc::new(RwLock::new(1));

let n = block_on(lock.write());

let cloned_lock = lock.clone();
let handle = thread::spawn(move || {
let mut guard = block_on(cloned_lock.write());
*guard = 2;
});

let n = n.downgrade();
assert_eq!(*n, 1);

drop(n);
handle.join().unwrap();
assert_eq!(*block_on(lock.read()), 2);
});
}
1 change: 1 addition & 0 deletions tokio/src/sync/tests/mod.rs
Expand Up @@ -12,4 +12,5 @@ cfg_loom! {
mod loom_oneshot;
mod loom_semaphore_batch;
mod loom_watch;
mod loom_rwlock;
}

0 comments on commit e804f88

Please sign in to comment.