-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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: RwLock::downgrade is not correct #2941
Comments
carllerche
added
C-bug
Category: This is a bug.
A-tokio
Area: The main tokio crate
labels
Oct 12, 2020
Hm, this seems to be related to some race in the batch semaphore. I assume the following test should pass but it fails: #[test]
fn maybe_race() {
loom::model(|| {
let semaphore = Arc::new(Semaphore::new(10));
semaphore
.try_acquire(10)
.expect("try_acquire should succeed; semaphore uncontended");
let semaphore2 = semaphore.clone();
let thread = thread::spawn(move || block_on(semaphore2.acquire(10)).unwrap());
semaphore.release(1);
// this fails as semaphore.available_permits() gives 0 ...
assert_eq!(1, semaphore.available_permits());
semaphore.release(9);
thread.join().unwrap();
assert_eq!(0, semaphore.available_permits());
})
} I am keen to take a look at that if I am not stepping on somebody's toes... :) UPDATE: Scratch the above. The test makes sense and should fail. The problem is elsewhere. Will submit a PR shortly. |
Go for it |
zaharidichev
added a commit
to zaharidichev/tokio
that referenced
this issue
Oct 14, 2020
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: tokio-rs#2941
carllerche
pushed a commit
that referenced
this issue
Oct 23, 2020
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I noticed CI can spuriously fail on this test.
The following loom test will fail:
The text was updated successfully, but these errors were encountered: