Skip to content

Commit acb3ec0

Browse files
committed
test and fix for rwlock unlock bug
1 parent a80821e commit acb3ec0

File tree

3 files changed

+50
-2
lines changed

3 files changed

+50
-2
lines changed

src/shims/sync.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::convert::TryInto;
22
use std::time::{Duration, SystemTime};
3+
use std::ops::Not;
34

45
use rustc_middle::ty::{layout::TyAndLayout, TyKind, TypeAndMut};
56
use rustc_target::abi::{LayoutOf, Size};
@@ -603,7 +604,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
603604

604605
if this.rwlock_reader_unlock(id, active_thread) {
605606
// The thread was a reader.
606-
if this.rwlock_is_locked(id) {
607+
if this.rwlock_is_locked(id).not() {
607608
// No more readers owning the lock. Give it to a writer if there
608609
// is any.
609610
this.rwlock_dequeue_and_lock_writer(id);

src/sync.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
282282
/// Lock by setting the writer that owns the lock.
283283
fn rwlock_writer_lock(&mut self, id: RwLockId, writer: ThreadId) {
284284
let this = self.eval_context_mut();
285-
assert!(!this.rwlock_is_locked(id), "the relock is already locked");
285+
assert!(!this.rwlock_is_locked(id), "the rwlock is already locked");
286286
this.machine.threads.sync.rwlocks[id].writer = Some(writer);
287287
}
288288

tests/run-pass/concurrency/sync.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,51 @@ fn check_once() {
267267
}
268268
}
269269

270+
fn check_rwlock_unlock_bug1() {
271+
// There was a bug where when un-read-locking an rwlock that still has other
272+
// readers waiting, we'd accidentally also let a writer in.
273+
// That caused an ICE.
274+
let l = Arc::new(RwLock::new(0));
275+
276+
let r1 = l.read().unwrap();
277+
let r2 = l.read().unwrap();
278+
279+
// Make a waiting writer.
280+
let l2 = l.clone();
281+
thread::spawn(move || {
282+
let mut w = l2.write().unwrap();
283+
*w += 1;
284+
});
285+
thread::yield_now();
286+
287+
drop(r1);
288+
assert_eq!(*r2, 0);
289+
thread::yield_now();
290+
thread::yield_now();
291+
thread::yield_now();
292+
assert_eq!(*r2, 0);
293+
drop(r2);
294+
}
295+
296+
fn check_rwlock_unlock_bug2() {
297+
// There was a bug where when un-read-locking an rwlock by letting the last reader leaver,
298+
// we'd forget to wake up a writer.
299+
// That meant the writer thread could never run again.
300+
let l = Arc::new(RwLock::new(0));
301+
302+
let r = l.read().unwrap();
303+
304+
// Make a waiting writer.
305+
let l2 = l.clone();
306+
let h = thread::spawn(move || {
307+
let _w = l2.write().unwrap();
308+
});
309+
thread::yield_now();
310+
311+
drop(r);
312+
h.join().unwrap();
313+
}
314+
270315
fn main() {
271316
check_barriers();
272317
check_conditional_variables_notify_one();
@@ -280,4 +325,6 @@ fn main() {
280325
multiple_send();
281326
send_on_sync();
282327
check_once();
328+
check_rwlock_unlock_bug1();
329+
check_rwlock_unlock_bug2();
283330
}

0 commit comments

Comments
 (0)