From b01d2babaf29b2798fa624676a1c9fcbcbc1e30a Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 28 Feb 2013 14:21:39 -0800 Subject: [PATCH 1/2] rt: Comment out an assert in rust_kernel. #4711 --- src/rt/rust_kernel.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rt/rust_kernel.cpp b/src/rt/rust_kernel.cpp index 4d2d6ad344cc9..761dbeade538b 100644 --- a/src/rt/rust_kernel.cpp +++ b/src/rt/rust_kernel.cpp @@ -342,8 +342,11 @@ rust_kernel::register_exit_function(spawn_fn runner, fn_env_pair *f) { assert(!at_exit_started && "registering at_exit function after exit"); if (at_exit_runner) { - assert(runner == at_exit_runner - && "there can be only one at_exit_runner"); + // FIXME #2912 Would be very nice to assert this but we can't because + // of the way coretest works (the test case ends up using its own + // function) + //assert(runner == at_exit_runner + // && "there can be only one at_exit_runner"); } at_exit_runner = runner; From 78d5091a4f09f0f7c613437e502db95b63a0c538 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 28 Feb 2013 15:20:40 -0800 Subject: [PATCH 2/2] core: Remove unwrap_shared_mutable_state. #4436 --- src/libcore/private.rs | 190 +---------------------------------------- src/libstd/arc.rs | 66 +------------- 2 files changed, 5 insertions(+), 251 deletions(-) diff --git a/src/libcore/private.rs b/src/libcore/private.rs index e4fab18966cad..5601964685e7b 100644 --- a/src/libcore/private.rs +++ b/src/libcore/private.rs @@ -107,20 +107,9 @@ fn compare_and_swap(address: &mut int, oldval: int, newval: int) -> bool { * Shared state & exclusive ARC ****************************************************************************/ -struct UnwrapProtoInner { - contents: Option<(comm::ChanOne<()>, comm::PortOne)>, -} - -// An unwrapper uses this protocol to communicate with the "other" task that -// drops the last refcount on an arc. Unfortunately this can't be a proper -// pipe protocol because the unwrapper has to access both stages at once. -type UnwrapProto = ~UnwrapProtoInner; - struct ArcData { mut count: libc::intptr_t, - mut unwrapper: int, // either a UnwrapProto or 0 - // FIXME(#3224) should be able to make this non-option to save memory, and - // in unwrap() use "let ~ArcData { data: result, _ } = thing" to unwrap it + // FIXME(#3224) should be able to make this non-option to save memory mut data: Option, } @@ -131,37 +120,13 @@ struct ArcDestruct { impl Drop for ArcDestruct{ fn finalize(&self) { unsafe { - if self.data.is_null() { - return; // Happens when destructing an unwrapper's handle. - } do task::unkillable { let data: ~ArcData = cast::reinterpret_cast(&self.data); let new_count = intrinsics::atomic_xsub(&mut data.count, 1) - 1; assert new_count >= 0; if new_count == 0 { - // Were we really last, or should we hand off to an - // unwrapper? It's safe to not xchg because the unwrapper - // will set the unwrap lock *before* dropping his/her - // reference. In effect, being here means we're the only - // *awake* task with the data. - if data.unwrapper != 0 { - let mut p: UnwrapProto = - cast::reinterpret_cast(&data.unwrapper); - let (message, response) = - option::swap_unwrap(&mut p.contents); - // Send 'ready' and wait for a response. - comm::send_one(message, ()); - // Unkillable wait. Message guaranteed to come. - if comm::recv_one(response) { - // Other task got the data. - cast::forget(data); - } else { - // Other task was killed. drop glue takes over. - } - } else { - // drop glue takes over. - } + // drop glue takes over. } else { cast::forget(data); } @@ -176,79 +141,6 @@ fn ArcDestruct(data: *libc::c_void) -> ArcDestruct { } } -pub unsafe fn unwrap_shared_mutable_state(rc: SharedMutableState) - -> T { - struct DeathThroes { - mut ptr: Option<~ArcData>, - mut response: Option>, - } - - impl Drop for DeathThroes{ - fn finalize(&self) { - unsafe { - let response = option::swap_unwrap(&mut self.response); - // In case we get killed early, we need to tell the person who - // tried to wake us whether they should hand-off the data to - // us. - if task::failing() { - comm::send_one(response, false); - // Either this swap_unwrap or the one below (at "Got - // here") ought to run. - cast::forget(option::swap_unwrap(&mut self.ptr)); - } else { - assert self.ptr.is_none(); - comm::send_one(response, true); - } - } - } - } - - do task::unkillable { - let ptr: ~ArcData = cast::reinterpret_cast(&rc.data); - let (p1,c1) = comm::oneshot(); // () - let (p2,c2) = comm::oneshot(); // bool - let mut server: UnwrapProto = ~UnwrapProtoInner { - contents: Some((c1,p2)) - }; - let serverp: int = cast::transmute(server); - // Try to put our server end in the unwrapper slot. - if compare_and_swap(&mut ptr.unwrapper, 0, serverp) { - // Got in. Step 0: Tell destructor not to run. We are now it. - rc.data = ptr::null(); - // Step 1 - drop our own reference. - let new_count = intrinsics::atomic_xsub(&mut ptr.count, 1) - 1; - //assert new_count >= 0; - if new_count == 0 { - // We were the last owner. Can unwrap immediately. - // Also we have to free the server endpoints. - let _server: UnwrapProto = cast::transmute(serverp); - option::swap_unwrap(&mut ptr.data) - // drop glue takes over. - } else { - // The *next* person who sees the refcount hit 0 will wake us. - let end_result = - DeathThroes { ptr: Some(ptr), - response: Some(c2) }; - let mut p1 = Some(p1); // argh - do task::rekillable { - comm::recv_one(option::swap_unwrap(&mut p1)); - } - // Got here. Back in the 'unkillable' without getting killed. - // Recover ownership of ptr, then take the data out. - let ptr = option::swap_unwrap(&mut end_result.ptr); - option::swap_unwrap(&mut ptr.data) - // drop glue takes over. - } - } else { - // Somebody else was trying to unwrap. Avoid guaranteed deadlock. - cast::forget(ptr); - // Also we have to free the (rejected) server endpoints. - let _server: UnwrapProto = cast::transmute(serverp); - fail!(~"Another task is already unwrapping this ARC!"); - } - } -} - /** * COMPLETELY UNSAFE. Used as a primitive for the safe versions in std::arc. * @@ -259,7 +151,7 @@ pub type SharedMutableState = ArcDestruct; pub unsafe fn shared_mutable_state(data: T) -> SharedMutableState { - let data = ~ArcData { count: 1, unwrapper: 0, data: Some(data) }; + let data = ~ArcData { count: 1, data: Some(data) }; unsafe { let ptr = cast::transmute(data); ArcDestruct(ptr) @@ -413,14 +305,6 @@ impl Exclusive { } } -// FIXME(#3724) make this a by-move method on the exclusive -pub fn unwrap_exclusive(arc: Exclusive) -> T { - let Exclusive { x: x } = arc; - let inner = unsafe { unwrap_shared_mutable_state(x) }; - let ExData { data: data, _ } = inner; - data -} - #[cfg(test)] pub mod tests { use core::option::{None, Some}; @@ -428,7 +312,7 @@ pub mod tests { use cell::Cell; use comm; use option; - use private::{exclusive, unwrap_exclusive}; + use private::exclusive; use result; use task; use uint; @@ -479,70 +363,4 @@ pub mod tests { assert *one == 1; } } - - #[test] - pub fn exclusive_unwrap_basic() { - let x = exclusive(~~"hello"); - assert unwrap_exclusive(x) == ~~"hello"; - } - - #[test] - pub fn exclusive_unwrap_contended() { - let x = exclusive(~~"hello"); - let x2 = Cell(x.clone()); - do task::spawn { - let x2 = x2.take(); - do x2.with |_hello| { } - task::yield(); - } - assert unwrap_exclusive(x) == ~~"hello"; - - // Now try the same thing, but with the child task blocking. - let x = exclusive(~~"hello"); - let x2 = Cell(x.clone()); - let mut res = None; - do task::task().future_result(|+r| res = Some(r)).spawn { - let x2 = x2.take(); - assert unwrap_exclusive(x2) == ~~"hello"; - } - // Have to get rid of our reference before blocking. - { let _x = x; } // FIXME(#3161) util::ignore doesn't work here - let res = option::swap_unwrap(&mut res); - res.recv(); - } - - #[test] #[should_fail] #[ignore(cfg(windows))] - pub fn exclusive_unwrap_conflict() { - let x = exclusive(~~"hello"); - let x2 = Cell(x.clone()); - let mut res = None; - do task::task().future_result(|+r| res = Some(r)).spawn { - let x2 = x2.take(); - assert unwrap_exclusive(x2) == ~~"hello"; - } - assert unwrap_exclusive(x) == ~~"hello"; - let res = option::swap_unwrap(&mut res); - // See #4689 for why this can't be just "res.recv()". - assert res.recv() == task::Success; - } - - #[test] #[ignore(cfg(windows))] - pub fn exclusive_unwrap_deadlock() { - // This is not guaranteed to get to the deadlock before being killed, - // but it will show up sometimes, and if the deadlock were not there, - // the test would nondeterministically fail. - let result = do task::try { - // a task that has two references to the same exclusive will - // deadlock when it unwraps. nothing to be done about that. - let x = exclusive(~~"hello"); - let x2 = x.clone(); - do task::spawn { - for 10.times { task::yield(); } // try to let the unwrapper go - fail!(); // punt it awake from its deadlock - } - let _z = unwrap_exclusive(x); - do x2.with |_hello| { } - }; - assert result.is_err(); - } } diff --git a/src/libstd/arc.rs b/src/libstd/arc.rs index f258e649122bd..264ea9d02787d 100644 --- a/src/libstd/arc.rs +++ b/src/libstd/arc.rs @@ -21,7 +21,7 @@ use core::cell::Cell; use core::pipes; use core::prelude::*; use core::private::{SharedMutableState, shared_mutable_state}; -use core::private::{clone_shared_mutable_state, unwrap_shared_mutable_state}; +use core::private::{clone_shared_mutable_state}; use core::private::{get_shared_mutable_state, get_shared_immutable_state}; use core::ptr; use core::task; @@ -104,20 +104,6 @@ pub fn clone(rc: &ARC) -> ARC { ARC { x: unsafe { clone_shared_mutable_state(&rc.x) } } } -/** - * Retrieve the data back out of the ARC. This function blocks until the - * reference given to it is the last existing one, and then unwrap the data - * instead of destroying it. - * - * If multiple tasks call unwrap, all but the first will fail. Do not call - * unwrap from a task that holds another reference to the same ARC; it is - * guaranteed to deadlock. - */ -pub fn unwrap(rc: ARC) -> T { - let ARC { x: x } = rc; - unsafe { unwrap_shared_mutable_state(x) } -} - impl Clone for ARC { fn clone(&self) -> ARC { clone(self) @@ -213,23 +199,6 @@ impl &MutexARC { } } -/** - * Retrieves the data, blocking until all other references are dropped, - * exactly as arc::unwrap. - * - * Will additionally fail if another task has failed while accessing the arc. - */ -// FIXME(#3724) make this a by-move method on the arc -pub fn unwrap_mutex_arc(arc: MutexARC) -> T { - let MutexARC { x: x } = arc; - let inner = unsafe { unwrap_shared_mutable_state(x) }; - let MutexARCInner { failed: failed, data: data, _ } = inner; - if failed { - fail!(~"Can't unwrap poisoned MutexARC - another task failed inside!") - } - data -} - // Common code for {mutex.access,rwlock.write}{,_cond}. #[inline(always)] #[doc(hidden)] @@ -411,24 +380,6 @@ impl &RWARC { } } -/** - * Retrieves the data, blocking until all other references are dropped, - * exactly as arc::unwrap. - * - * Will additionally fail if another task has failed while accessing the arc - * in write mode. - */ -// FIXME(#3724) make this a by-move method on the arc -pub fn unwrap_rw_arc(arc: RWARC) -> T { - let RWARC { x: x, _ } = arc; - let inner = unsafe { unwrap_shared_mutable_state(x) }; - let RWARCInner { failed: failed, data: data, _ } = inner; - if failed { - fail!(~"Can't unwrap poisoned RWARC - another task failed inside!") - } - data -} - // Borrowck rightly complains about immutably aliasing the rwlock in order to // lock it. This wraps the unsafety, with the justification that the 'lock' // field is never overwritten; only 'failed' and 'data'. @@ -585,21 +536,6 @@ mod tests { } } #[test] #[should_fail] #[ignore(cfg(windows))] - pub fn test_mutex_arc_unwrap_poison() { - let arc = MutexARC(1); - let arc2 = ~(&arc).clone(); - let (p, c) = comm::stream(); - do task::spawn || { - do arc2.access |one| { - c.send(()); - assert *one == 2; - } - } - let _ = p.recv(); - let one = unwrap_mutex_arc(arc); - assert one == 1; - } - #[test] #[should_fail] #[ignore(cfg(windows))] pub fn test_rw_arc_poison_wr() { let arc = ~RWARC(1); let arc2 = ~arc.clone();