diff --git a/src/libstd/comm/mod.rs b/src/libstd/comm/mod.rs index b2521f2978a..18857e221fa 100644 --- a/src/libstd/comm/mod.rs +++ b/src/libstd/comm/mod.rs @@ -600,20 +600,22 @@ impl Clone for Sender { let (packet, sleeper) = match *unsafe { self.inner() } { Oneshot(ref p) => { let a = Arc::new(Unsafe::new(shared::Packet::new())); - match unsafe { - (*p.get()).upgrade(Receiver::new(Shared(a.clone()))) - } { - oneshot::UpSuccess | oneshot::UpDisconnected => (a, None), - oneshot::UpWoke(task) => (a, Some(task)) + unsafe { + (*a.get()).postinit_lock(); + match (*p.get()).upgrade(Receiver::new(Shared(a.clone()))) { + oneshot::UpSuccess | oneshot::UpDisconnected => (a, None), + oneshot::UpWoke(task) => (a, Some(task)) + } } } Stream(ref p) => { let a = Arc::new(Unsafe::new(shared::Packet::new())); - match unsafe { - (*p.get()).upgrade(Receiver::new(Shared(a.clone()))) - } { - stream::UpSuccess | stream::UpDisconnected => (a, None), - stream::UpWoke(task) => (a, Some(task)), + unsafe { + (*a.get()).postinit_lock(); + match (*p.get()).upgrade(Receiver::new(Shared(a.clone()))) { + stream::UpSuccess | stream::UpDisconnected => (a, None), + stream::UpWoke(task) => (a, Some(task)), + } } } Shared(ref p) => { diff --git a/src/libstd/comm/shared.rs b/src/libstd/comm/shared.rs index 8aef2ec80a8..3fde584a46f 100644 --- a/src/libstd/comm/shared.rs +++ b/src/libstd/comm/shared.rs @@ -66,7 +66,8 @@ pub enum Failure { } impl Packet { - // Creation of a packet *must* be followed by a call to inherit_blocker + // Creation of a packet *must* be followed by a call to postinit_lock + // and later by inherit_blocker pub fn new() -> Packet { let p = Packet { queue: mpsc::Queue::new(), @@ -78,11 +79,18 @@ impl Packet { sender_drain: atomics::AtomicInt::new(0), select_lock: unsafe { NativeMutex::new() }, }; - // see comments in inherit_blocker about why we grab this lock - unsafe { p.select_lock.lock_noguard() } return p; } + // This function should be used after newly created Packet + // was wrapped with an Arc + // In other case mutex data will be duplicated while clonning + // and that could cause problems on platforms where it is + // represented by opaque data structure + pub fn postinit_lock(&mut self) { + unsafe { self.select_lock.lock_noguard() } + } + // This function is used at the creation of a shared packet to inherit a // previously blocked task. This is done to prevent spurious wakeups of // tasks in select(). diff --git a/src/libstd/unstable/mutex.rs b/src/libstd/unstable/mutex.rs index 760214eb8f8..04da7dab6c6 100644 --- a/src/libstd/unstable/mutex.rs +++ b/src/libstd/unstable/mutex.rs @@ -98,6 +98,7 @@ impl StaticNativeMutex { /// /// Note that a mutex created in this way needs to be explicit /// freed with a call to `destroy` or it will leak. + /// Also it is important to avoid locking until mutex has stopped moving pub unsafe fn new() -> StaticNativeMutex { StaticNativeMutex { inner: imp::Mutex::new() } } @@ -172,6 +173,7 @@ impl NativeMutex { /// /// The user must be careful to ensure the mutex is not locked when its is /// being destroyed. + /// Also it is important to avoid locking until mutex has stopped moving pub unsafe fn new() -> NativeMutex { NativeMutex { inner: StaticNativeMutex::new() } } @@ -262,7 +264,6 @@ mod imp { use libc; use self::os::{PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER, pthread_mutex_t, pthread_cond_t}; - use mem; use ty::Unsafe; use kinds::marker; @@ -294,6 +295,7 @@ mod imp { static __PTHREAD_MUTEX_SIZE__: uint = 40; #[cfg(target_arch = "x86")] static __PTHREAD_COND_SIZE__: uint = 24; + static _PTHREAD_MUTEX_SIG_init: libc::c_long = 0x32AAABA7; static _PTHREAD_COND_SIG_init: libc::c_long = 0x3CB0B1BB; @@ -389,14 +391,14 @@ mod imp { impl Mutex { pub unsafe fn new() -> Mutex { + // As mutex might be moved and address is changing it + // is better to avoid initialization of potentially + // opaque OS data before it landed let m = Mutex { - lock: Unsafe::new(mem::zeroed()), - cond: Unsafe::new(mem::zeroed()), + lock: Unsafe::new(PTHREAD_MUTEX_INITIALIZER), + cond: Unsafe::new(PTHREAD_COND_INITIALIZER), }; - pthread_mutex_init(m.lock.get(), 0 as *libc::c_void); - pthread_cond_init(m.cond.get(), 0 as *libc::c_void); - return m; } @@ -416,11 +418,7 @@ mod imp { } extern { - fn pthread_mutex_init(lock: *mut pthread_mutex_t, - attr: *pthread_mutexattr_t) -> libc::c_int; fn pthread_mutex_destroy(lock: *mut pthread_mutex_t) -> libc::c_int; - fn pthread_cond_init(cond: *mut pthread_cond_t, - attr: *pthread_condattr_t) -> libc::c_int; fn pthread_cond_destroy(cond: *mut pthread_cond_t) -> libc::c_int; fn pthread_mutex_lock(lock: *mut pthread_mutex_t) -> libc::c_int; fn pthread_mutex_trylock(lock: *mut pthread_mutex_t) -> libc::c_int;