From 589ee65fd4ea311beeb42ebdc5e6a4b6960bd303 Mon Sep 17 00:00:00 2001 From: Ben Blum Date: Tue, 14 Aug 2012 13:32:41 -0400 Subject: [PATCH] Add rw_arc.downgrade() + std and cfail tests. Tons of region FIXMEs... (cf #2282, #3154) --- src/libcore/ptr.rs | 2 +- src/libcore/unsafe.rs | 12 + src/libstd/arc.rs | 231 +++++++++++++++++- src/libstd/sync.rs | 52 ++-- .../arc-rw-cond-shouldnt-escape.rs | 11 + .../arc-rw-read-mode-shouldnt-escape.rs | 12 + .../arc-rw-write-mode-cond-shouldnt-escape.rs | 13 + .../arc-rw-write-mode-shouldnt-escape.rs | 12 + .../sync-rwlock-cond-shouldnt-escape.rs | 11 + .../sync-rwlock-read-mode-shouldnt-escape.rs | 12 + ...-rwlock-write-mode-cond-shouldnt-escape.rs | 13 + .../sync-rwlock-write-mode-shouldnt-escape.rs | 12 + 12 files changed, 365 insertions(+), 28 deletions(-) create mode 100644 src/test/compile-fail/arc-rw-cond-shouldnt-escape.rs create mode 100644 src/test/compile-fail/arc-rw-read-mode-shouldnt-escape.rs create mode 100644 src/test/compile-fail/arc-rw-write-mode-cond-shouldnt-escape.rs create mode 100644 src/test/compile-fail/arc-rw-write-mode-shouldnt-escape.rs create mode 100644 src/test/compile-fail/sync-rwlock-cond-shouldnt-escape.rs create mode 100644 src/test/compile-fail/sync-rwlock-read-mode-shouldnt-escape.rs create mode 100644 src/test/compile-fail/sync-rwlock-write-mode-cond-shouldnt-escape.rs create mode 100644 src/test/compile-fail/sync-rwlock-write-mode-shouldnt-escape.rs diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index c06f3f1d3a4e..b7b63af0afbb 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -152,7 +152,7 @@ fn to_uint(thing: &T) -> uint unsafe { /// Determine if two borrowed pointers point to the same thing. #[inline(always)] -fn ref_eq(thing: &T, other: &T) -> bool { +fn ref_eq(thing: &a/T, other: &b/T) -> bool { to_uint(thing) == to_uint(other) } diff --git a/src/libcore/unsafe.rs b/src/libcore/unsafe.rs index f8c6074abcf0..f945cb8ad1d8 100644 --- a/src/libcore/unsafe.rs +++ b/src/libcore/unsafe.rs @@ -1,6 +1,7 @@ //! Unsafe operations export reinterpret_cast, forget, bump_box_refcount, transmute; +export transmute_mut, transmute_immut, transmute_region, transmute_mut_region; export SharedMutableState, shared_mutable_state, clone_shared_mutable_state; export get_shared_mutable_state, get_shared_immutable_state; @@ -53,6 +54,17 @@ unsafe fn transmute(-thing: L) -> G { return newthing; } +/// Coerce an immutable reference to be mutable. +unsafe fn transmute_mut(+ptr: &T) -> &mut T { transmute(ptr) } +/// Coerce a mutable reference to be immutable. +unsafe fn transmute_immut(+ptr: &mut T) -> &T { transmute(ptr) } +/// Coerce a borrowed pointer to have an arbitrary associated region. +unsafe fn transmute_region(+ptr: &a/T) -> &b/T { transmute(ptr) } +/// Coerce a borrowed mutable pointer to have an arbitrary associated region. +unsafe fn transmute_mut_region(+ptr: &a/mut T) -> &b/mut T { + transmute(ptr) +} + /**************************************************************************** * Shared state & exclusive ARC ****************************************************************************/ diff --git a/src/libstd/arc.rs b/src/libstd/arc.rs index 847e08083ef4..8ab74444f6f1 100644 --- a/src/libstd/arc.rs +++ b/src/libstd/arc.rs @@ -10,7 +10,7 @@ import sync; import sync::{mutex, rwlock}; export arc, clone, get; -export condvar, mutex_arc, rw_arc; +export condvar, mutex_arc, rw_arc, rw_write_mode, rw_read_mode; /// As sync::condvar, a mechanism for unlock-and-descheduling and signalling. struct condvar { is_mutex: bool; failed: &mut bool; cond: &sync::condvar; } @@ -136,10 +136,11 @@ impl &mutex_arc { &condvar { is_mutex: true, failed: &mut state.failed, cond: cond }) */ - // XXX: Working around two seeming region bugs here - let fref = unsafe { unsafe::reinterpret_cast(&mut state.failed) }; + // FIXME(#2282) region variance + let fref = + unsafe { unsafe::transmute_mut_region(&mut state.failed) }; let cvar = condvar { is_mutex: true, failed: fref, cond: cond }; - blk(&mut state.data, unsafe { unsafe::reinterpret_cast(&cvar) } ) + blk(&mut state.data, unsafe { unsafe::transmute_region(&cvar) } ) } } } @@ -227,10 +228,12 @@ impl &rw_arc { &condvar { is_mutex: false, failed: &mut state.failed, cond: cond }) */ - // XXX: Working around two seeming region bugs here - let fref = unsafe { unsafe::reinterpret_cast(&mut state.failed) }; + // FIXME(#2282): Need region variance to use the commented-out + // code above instead of this casting mess + let fref = + unsafe { unsafe::transmute_mut_region(&mut state.failed) }; let cvar = condvar { is_mutex: false, failed: fref, cond: cond }; - blk(&mut state.data, unsafe { unsafe::reinterpret_cast(&cvar) } ) + blk(&mut state.data, unsafe { unsafe::transmute_region(&cvar) } ) } } /** @@ -249,6 +252,52 @@ impl &rw_arc { blk(&state.data) } } + + /** + * As write(), but with the ability to atomically 'downgrade' the lock. + * See sync::rwlock.write_downgrade(). The rw_write_mode token must be + * used to obtain the &mut T, and can be transformed into a rw_read_mode + * token by calling downgrade(), after which a &T can be obtained instead. + * ~~~ + * do arc.write_downgrade |write_mode| { + * do (&write_mode).write_cond |state, condvar| { + * ... exclusive access with mutable state ... + * } + * let read_mode = arc.downgrade(write_mode); + * do (&read_mode).read |state| { + * ... shared access with immutable state ... + * } + * } + * ~~~ + */ + fn write_downgrade(blk: fn(+rw_write_mode) -> U) -> U { + let state = unsafe { get_shared_mutable_state(&self.x) }; + do borrow_rwlock(state).write_downgrade |write_mode| { + check_poison(false, state.failed); + // FIXME(#2282) need region variance to avoid having to cast here + let (data,failed) = + unsafe { (unsafe::transmute_mut_region(&mut state.data), + unsafe::transmute_mut_region(&mut state.failed)) }; + blk(rw_write_mode((data, write_mode, poison_on_fail(failed)))) + } + } + + /// To be called inside of the write_downgrade block. + fn downgrade(+token: rw_write_mode) -> rw_read_mode { + // The rwlock should assert that the token belongs to us for us. + let state = unsafe { get_shared_immutable_state(&self.x) }; + let rw_write_mode((data, t, _poison)) = token; + // Let readers in + let new_token = (&state.lock).downgrade(t); + // Whatever region the input reference had, it will be safe to use + // the same region for the output reference. (The only 'unsafe' part + // of this cast is removing the mutability.) + let new_data = unsafe { unsafe::transmute_immut(data) }; + // Downgrade ensured the token belonged to us. Just a sanity check. + assert ptr::ref_eq(&state.data, new_data); + // Produce new token + rw_read_mode((new_data, new_token)) + } } // Borrowck rightly complains about immutably aliasing the rwlock in order to @@ -258,6 +307,58 @@ fn borrow_rwlock(state: &mut rw_arc_inner) -> &rwlock { unsafe { unsafe::reinterpret_cast(&state.lock) } } +// FIXME (#3154) ice with struct/& prevents these from being structs. + +/// The "write permission" token used for rw_arc.write_downgrade(). +enum rw_write_mode = + (&mut T, sync::rwlock_write_mode, poison_on_fail); +/// The "read permission" token used for rw_arc.write_downgrade(). +enum rw_read_mode = (&T, sync::rwlock_read_mode); + +impl &rw_write_mode { + /// Access the pre-downgrade rw_arc in write mode. + fn write(blk: fn(x: &mut T) -> U) -> U { + match *self { + rw_write_mode((data, ref token, _)) => { + // FIXME(#2282) cast to avoid region invariance + let mode = unsafe { unsafe::transmute_region(token) }; + do mode.write { + blk(data) + } + } + } + } + /// Access the pre-downgrade rw_arc in write mode with a condvar. + fn write_cond(blk: fn(x: &x/mut T, c: &c/condvar) -> U) -> U { + match *self { + rw_write_mode((data, ref token, ref poison)) => { + // FIXME(#2282) cast to avoid region invariance + let mode = unsafe { unsafe::transmute_region(token) }; + do mode.write_cond |cond| { + let cvar = condvar { + is_mutex: false, failed: poison.failed, + cond: unsafe { unsafe::reinterpret_cast(cond) } }; + // FIXME(#2282) region variance would avoid having to cast + blk(data, &cvar) + } + } + } + } +} + +impl &rw_read_mode { + /// Access the post-downgrade rwlock in read mode. + fn read(blk: fn(x: &T) -> U) -> U { + match *self { + rw_read_mode((data, ref token)) => { + // FIXME(#2282) cast to avoid region invariance + let mode = unsafe { unsafe::transmute_region(token) }; + do mode.read { blk(data) } + } + } + } +} + /**************************************************************************** * Tests ****************************************************************************/ @@ -374,6 +475,23 @@ mod tests { assert *one == 1; } } + #[test] #[should_fail] #[ignore(cfg(windows))] + fn test_rw_arc_poison_dw() { + let arc = ~rw_arc(1); + let arc2 = ~arc.clone(); + do task::try { + do arc2.write_downgrade |write_mode| { + // FIXME(#2282) + let mode = unsafe { unsafe::transmute_region(&write_mode) }; + do mode.write |one| { + assert *one == 2; + } + } + }; + do arc.write |one| { + assert *one == 1; + } + } #[test] #[ignore(cfg(windows))] fn test_rw_arc_no_poison_rr() { let arc = ~rw_arc(1); @@ -400,7 +518,24 @@ mod tests { assert *one == 1; } } - + #[test] #[ignore(cfg(windows))] + fn test_rw_arc_no_poison_dr() { + let arc = ~rw_arc(1); + let arc2 = ~arc.clone(); + do task::try { + do arc2.write_downgrade |write_mode| { + let read_mode = arc2.downgrade(write_mode); + // FIXME(#2282) + let mode = unsafe { unsafe::transmute_region(&read_mode) }; + do mode.read |one| { + assert *one == 2; + } + } + }; + do arc.write |one| { + assert *one == 1; + } + } #[test] fn test_rw_arc() { let arc = ~rw_arc(0); @@ -434,4 +569,84 @@ mod tests { p.recv(); do arc.read |num| { assert *num == 10; } } + #[test] + fn test_rw_downgrade() { + // (1) A downgrader gets in write mode and does cond.wait. + // (2) A writer gets in write mode, sets state to 42, and does signal. + // (3) Downgrader wakes, sets state to 31337. + // (4) tells writer and all other readers to contend as it downgrades. + // (5) Writer attempts to set state back to 42, while downgraded task + // and all reader tasks assert that it's 31337. + let arc = ~rw_arc(0); + + // Reader tasks + let mut reader_convos = ~[]; + for 10.times { + let ((rc1,rp1),(rc2,rp2)) = (pipes::stream(),pipes::stream()); + vec::push(reader_convos, (rc1,rp2)); + let arcn = ~arc.clone(); + do task::spawn { + rp1.recv(); // wait for downgrader to give go-ahead + do arcn.read |state| { + assert *state == 31337; + rc2.send(()); + } + } + } + + // Writer task + let arc2 = ~arc.clone(); + let ((wc1,wp1),(wc2,wp2)) = (pipes::stream(),pipes::stream()); + do task::spawn { + wp1.recv(); + do arc2.write_cond |state, cond| { + assert *state == 0; + *state = 42; + cond.signal(); + } + wp1.recv(); + do arc2.write |state| { + // This shouldn't happen until after the downgrade read + // section, and all other readers, finish. + assert *state == 31337; + *state = 42; + } + wc2.send(()); + } + + // Downgrader (us) + do arc.write_downgrade |write_mode| { + // FIXME(#2282) + let mode = unsafe { unsafe::transmute_region(&write_mode) }; + do mode.write_cond |state, cond| { + wc1.send(()); // send to another writer who will wake us up + while *state == 0 { + cond.wait(); + } + assert *state == 42; + *state = 31337; + // send to other readers + for vec::each(reader_convos) |x| { + match x { + (rc, _) => rc.send(()), + } + } + } + let read_mode = arc.downgrade(write_mode); + // FIXME(#2282) + let mode = unsafe { unsafe::transmute_region(&read_mode) }; + do mode.read |state| { + // complete handshake with other readers + for vec::each(reader_convos) |x| { + match x { + (_, rp) => rp.recv(), + } + } + wc1.send(()); // tell writer to try again + assert *state == 31337; + } + } + + wp2.recv(); // complete handshake with writer + } } diff --git a/src/libstd/sync.rs b/src/libstd/sync.rs index 20aa6846bcbd..b2dd55fdcad9 100644 --- a/src/libstd/sync.rs +++ b/src/libstd/sync.rs @@ -5,7 +5,7 @@ * in std. */ -export condvar, semaphore, mutex, rwlock; +export condvar, semaphore, mutex, rwlock, rwlock_write_mode, rwlock_read_mode; // FIXME (#3119) This shouldn't be a thing exported from core. import unsafe::{Exclusive, exclusive}; @@ -387,16 +387,17 @@ impl &rwlock { * the meantime (such as unlocking and then re-locking as a reader would * do). The block takes a "write mode token" argument, which can be * transformed into a "read mode token" by calling downgrade(). Example: - * - * do lock.write_downgrade |write_mode| { - * do (&write_mode).write_cond |condvar| { - * ... exclusive access ... - * } - * let read_mode = lock.downgrade(write_mode); - * do (&read_mode).read { - * ... shared access ... - * } + * ~~~ + * do lock.write_downgrade |write_mode| { + * do (&write_mode).write_cond |condvar| { + * ... exclusive access ... * } + * let read_mode = lock.downgrade(write_mode); + * do (&read_mode).read { + * ... shared access ... + * } + * } + * ~~~ */ fn write_downgrade(blk: fn(+rwlock_write_mode) -> U) -> U { // Implementation slightly different from the slicker 'write's above. @@ -413,6 +414,7 @@ impl &rwlock { blk(rwlock_write_mode { lock: self }) } + /// To be called inside of the write_downgrade block. fn downgrade(+token: rwlock_write_mode) -> rwlock_read_mode { if !ptr::ref_eq(self, token.lock) { fail ~"Can't downgrade() with a different rwlock's write_mode!"; @@ -498,8 +500,7 @@ struct rwlock_write_mode { lock: &rwlock; drop { } } /// The "read permission" token used for rwlock.write_downgrade(). struct rwlock_read_mode { priv lock: &rwlock; drop { } } -// FIXME(#3145) XXX Region invariance forbids "mode.write(blk)" -impl rwlock_write_mode { +impl &rwlock_write_mode { /// Access the pre-downgrade rwlock in write mode. fn write(blk: fn() -> U) -> U { blk() } /// Access the pre-downgrade rwlock in write mode with a condvar. @@ -507,7 +508,7 @@ impl rwlock_write_mode { blk(&condvar { sem: &self.lock.access_lock }) } } -impl rwlock_read_mode { +impl &rwlock_read_mode { /// Access the post-downgrade rwlock in read mode. fn read(blk: fn() -> U) -> U { blk() } } @@ -777,9 +778,19 @@ mod tests { match mode { read => x.read(blk), write => x.write(blk), - downgrade => do x.write_downgrade |mode| { mode.write(blk); }, + downgrade => + do x.write_downgrade |mode| { + // FIXME(#2282) + let mode = unsafe { unsafe::transmute_region(&mode) }; + mode.write(blk); + }, downgrade_read => - do x.write_downgrade |mode| { x.downgrade(mode).read(blk); }, + do x.write_downgrade |mode| { + let mode = x.downgrade(mode); + // FIXME(#2282) + let mode = unsafe { unsafe::transmute_region(&mode) }; + mode.read(blk); + }, } } #[cfg(test)] @@ -922,7 +933,11 @@ mod tests { // Much like the mutex broadcast test. Downgrade-enabled. fn lock_cond(x: &rwlock, downgrade: bool, blk: fn(c: &condvar)) { if downgrade { - do x.write_downgrade |mode| { mode.write_cond(blk) } + do x.write_downgrade |mode| { + // FIXME(#2282) + let mode = unsafe { unsafe::transmute_region(&mode) }; + mode.write_cond(blk) + } } else { x.write_cond(blk) } @@ -1009,9 +1024,8 @@ mod tests { do x.write_downgrade |xwrite| { let mut xopt = some(xwrite); do y.write_downgrade |_ywrite| { - do y.downgrade(option::swap_unwrap(&mut xopt)).read { - error!("oops, y.downgrade(x) should have failed!"); - } + y.downgrade(option::swap_unwrap(&mut xopt)); + error!("oops, y.downgrade(x) should have failed!"); } } } diff --git a/src/test/compile-fail/arc-rw-cond-shouldnt-escape.rs b/src/test/compile-fail/arc-rw-cond-shouldnt-escape.rs new file mode 100644 index 000000000000..d8b3fd3e8b02 --- /dev/null +++ b/src/test/compile-fail/arc-rw-cond-shouldnt-escape.rs @@ -0,0 +1,11 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::arc; +fn main() { + let x = ~arc::rw_arc(1); + let mut y = none; + do x.write_cond |_one, cond| { + y = some(cond); + } + option::unwrap(y).wait(); +} diff --git a/src/test/compile-fail/arc-rw-read-mode-shouldnt-escape.rs b/src/test/compile-fail/arc-rw-read-mode-shouldnt-escape.rs new file mode 100644 index 000000000000..8ba84bf85dc4 --- /dev/null +++ b/src/test/compile-fail/arc-rw-read-mode-shouldnt-escape.rs @@ -0,0 +1,12 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::arc; +fn main() { + let x = ~arc::rw_arc(1); + let mut y = none; + do x.write_downgrade |write_mode| { + y = some(x.downgrade(write_mode)); + } + // Adding this line causes a method unification failure instead + // do (&option::unwrap(y)).read |state| { assert *state == 1; } +} diff --git a/src/test/compile-fail/arc-rw-write-mode-cond-shouldnt-escape.rs b/src/test/compile-fail/arc-rw-write-mode-cond-shouldnt-escape.rs new file mode 100644 index 000000000000..3584512c28e4 --- /dev/null +++ b/src/test/compile-fail/arc-rw-write-mode-cond-shouldnt-escape.rs @@ -0,0 +1,13 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::arc; +fn main() { + let x = ~arc::rw_arc(1); + let mut y = none; + do x.write_downgrade |write_mode| { + do (&write_mode).write_cond |_one, cond| { + y = some(cond); + } + } + option::unwrap(y).wait(); +} diff --git a/src/test/compile-fail/arc-rw-write-mode-shouldnt-escape.rs b/src/test/compile-fail/arc-rw-write-mode-shouldnt-escape.rs new file mode 100644 index 000000000000..e4ecef47476a --- /dev/null +++ b/src/test/compile-fail/arc-rw-write-mode-shouldnt-escape.rs @@ -0,0 +1,12 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::arc; +fn main() { + let x = ~arc::rw_arc(1); + let mut y = none; + do x.write_downgrade |write_mode| { + y = some(write_mode); + } + // Adding this line causes a method unification failure instead + // do (&option::unwrap(y)).write |state| { assert *state == 1; } +} diff --git a/src/test/compile-fail/sync-rwlock-cond-shouldnt-escape.rs b/src/test/compile-fail/sync-rwlock-cond-shouldnt-escape.rs new file mode 100644 index 000000000000..2b76279a4abf --- /dev/null +++ b/src/test/compile-fail/sync-rwlock-cond-shouldnt-escape.rs @@ -0,0 +1,11 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::sync; +fn main() { + let x = ~sync::rwlock(); + let mut y = none; + do x.write_cond |cond| { + y = some(cond); + } + option::unwrap(y).wait(); +} diff --git a/src/test/compile-fail/sync-rwlock-read-mode-shouldnt-escape.rs b/src/test/compile-fail/sync-rwlock-read-mode-shouldnt-escape.rs new file mode 100644 index 000000000000..ac04671ee372 --- /dev/null +++ b/src/test/compile-fail/sync-rwlock-read-mode-shouldnt-escape.rs @@ -0,0 +1,12 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::sync; +fn main() { + let x = ~sync::rwlock(); + let mut y = none; + do x.write_downgrade |write_mode| { + y = some(x.downgrade(write_mode)); + } + // Adding this line causes a method unification failure instead + // do (&option::unwrap(y)).read { } +} diff --git a/src/test/compile-fail/sync-rwlock-write-mode-cond-shouldnt-escape.rs b/src/test/compile-fail/sync-rwlock-write-mode-cond-shouldnt-escape.rs new file mode 100644 index 000000000000..7f60342b9994 --- /dev/null +++ b/src/test/compile-fail/sync-rwlock-write-mode-cond-shouldnt-escape.rs @@ -0,0 +1,13 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::sync; +fn main() { + let x = ~sync::rwlock(); + let mut y = none; + do x.write_downgrade |write_mode| { + do (&write_mode).write_cond |cond| { + y = some(cond); + } + } + option::unwrap(y).wait(); +} diff --git a/src/test/compile-fail/sync-rwlock-write-mode-shouldnt-escape.rs b/src/test/compile-fail/sync-rwlock-write-mode-shouldnt-escape.rs new file mode 100644 index 000000000000..f7571d11ac8a --- /dev/null +++ b/src/test/compile-fail/sync-rwlock-write-mode-shouldnt-escape.rs @@ -0,0 +1,12 @@ +// error-pattern: reference is not valid outside of its lifetime +use std; +import std::sync; +fn main() { + let x = ~sync::rwlock(); + let mut y = none; + do x.write_downgrade |write_mode| { + y = some(write_mode); + } + // Adding this line causes a method unification failure instead + // do (&option::unwrap(y)).write { } +}