From 41b65d39ab5b4fae52cbcc370b464ba370bedb87 Mon Sep 17 00:00:00 2001 From: Valerii Hiora Date: Sat, 24 May 2014 08:01:23 +0300 Subject: [PATCH] Fixes problems on systems with opaque mutex On some systems (iOS for example) mutex is represented by opaque data structure which doesn't play well with simple data copy. Therefore mutex should be initialized from magic static value and filled by OS only when it landed RC. --- src/libstd/comm/mod.rs | 22 ++++++++++++---------- src/libstd/comm/shared.rs | 14 +++++++++++--- src/libstd/unstable/mutex.rs | 18 ++++++++---------- 3 files changed, 31 insertions(+), 23 deletions(-) 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;