diff --git a/src/libcore/sync.rs b/src/libcore/sync.rs index 744744831178..fba9569c304a 100644 --- a/src/libcore/sync.rs +++ b/src/libcore/sync.rs @@ -162,7 +162,6 @@ impl condvar { // Release lock, 'atomically' enqueuing ourselves in so doing. do (**self.sem).with |state| { // Drop the lock. - // FIXME(#3145) investigate why factoring doesn't compile. state.count += 1; if state.count <= 0 { signal_waitqueue(&state.waiters); @@ -343,34 +342,41 @@ impl &rwlock { unsafe { do task::unkillable { (&self.order_lock).acquire(); - (&self.access_lock).acquire(); - (&self.order_lock).release(); + do (&self.access_lock).access { + (&self.order_lock).release(); + task::rekillable(blk) + } } } - let _z = rwlock_release_write(self); - blk() } /** * As write(), but also with a handle to a condvar. Waiting on this * condvar will allow readers and writers alike to take the rwlock before - * the waiting task is signalled. + * the waiting task is signalled. (Note: a writer that waited and then + * was signalled might reacquire the lock before other waiting writers.) */ - fn write_cond(_blk: fn(condvar) -> U) -> U { - fail ~"Need implement lock order lock before access lock"; + fn write_cond(blk: fn(condvar) -> U) -> U { + // NB: You might think I should thread the order_lock into the cond + // wait call, so that it gets waited on before access_lock gets + // reacquired upon being woken up. However, (a) this would be not + // pleasant to implement (and would mandate a new 'rw_cond' type) and + // (b) I think violating no-starvation in that case is appropriate. + unsafe { + do task::unkillable { + (&self.order_lock).acquire(); + do (&self.access_lock).access_cond |cond| { + (&self.order_lock).release(); + do task::rekillable { blk(cond) } + } + } + } } // to-do implement downgrade } -// FIXME(#3136) should go inside of write() and read() respectively -struct rwlock_release_write { - lock: &rwlock; - new(lock: &rwlock) { self.lock = lock; } - drop unsafe { - do task::unkillable { (&self.lock.access_lock).release(); } - } -} +// FIXME(#3136) should go inside of read() struct rwlock_release_read { lock: &rwlock; new(lock: &rwlock) { self.lock = lock; } @@ -708,6 +714,41 @@ mod tests { let _ = p1.recv(); } } + #[test] + fn test_rwlock_cond_wait() { + // As test_mutex_cond_wait above. + let x = ~rwlock(); + + // Child wakes up parent + do x.write_cond |cond| { + let x2 = ~x.clone(); + do task::spawn { + do x2.write_cond |cond| { + let woken = cond.signal(); + assert woken; + } + } + cond.wait(); + } + // Parent wakes up child + let (chan,port) = pipes::stream(); + let x3 = ~x.clone(); + do task::spawn { + do x3.write_cond |cond| { + chan.send(()); + cond.wait(); + chan.send(()); + } + } + let _ = port.recv(); // Wait until child gets in the rwlock + do x.read { } // Must be able to get in as a reader in the meantime + do x.write_cond |cond| { // Or as another writer + let woken = cond.signal(); + assert woken; + } + let _ = port.recv(); // Wait until child wakes up + do x.read { } // Just for good measure + } #[cfg(test)] #[ignore(cfg(windows))] fn rwlock_kill_helper(reader1: bool, reader2: bool) { // Mutex must get automatically unlocked if failed/killed within. diff --git a/src/libcore/task.rs b/src/libcore/task.rs index f80dd309c7b2..67e6dd6125c1 100644 --- a/src/libcore/task.rs +++ b/src/libcore/task.rs @@ -56,7 +56,7 @@ export try; export yield; export failing; export get_task; -export unkillable; +export unkillable, rekillable; export atomically; export local_data_key; @@ -572,7 +572,7 @@ fn get_task() -> task { * } * ~~~ */ -unsafe fn unkillable(f: fn()) { +unsafe fn unkillable(f: fn() -> U) -> U { class allow_failure { let t: *rust_task; new(t: *rust_task) { self.t = t; } @@ -582,7 +582,21 @@ unsafe fn unkillable(f: fn()) { let t = rustrt::rust_get_task(); let _allow_failure = allow_failure(t); rustrt::rust_task_inhibit_kill(t); - f(); + f() +} + +/// The inverse of unkillable. Only ever to be used nested in unkillable(). +unsafe fn rekillable(f: fn() -> U) -> U { + class disallow_failure { + let t: *rust_task; + new(t: *rust_task) { self.t = t; } + drop { rustrt::rust_task_inhibit_kill(self.t); } + } + + let t = rustrt::rust_get_task(); + let _allow_failure = disallow_failure(t); + rustrt::rust_task_allow_kill(t); + f() } /**