Skip to content
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

sync: add mem::forget to RwLockWriteGuard::downgrade. #2957

Merged
merged 2 commits into from Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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");
carllerche marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not look like loom_rwlock.rs is being included here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not. I was not sure whether this is intentional or not. Will include

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;
}