auto merge of #14396 : vhbit/rust/opaque-mutex, r=alexcrichton

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.

Initially written for iOS but since landing iOS support might require quite a lot of time I think it is better to split parts which aren't directly related to iOS and merge them in

Author: bors

src/libstd/comm/mod.rs

@@ -600,20 +600,22 @@ impl<T: Send> Clone for Sender<T> {
         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) => {

src/libstd/comm/shared.rs

@@ -66,7 +66,8 @@ pub enum Failure {
 }
 
 impl<T: Send> Packet<T> {
-    // 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<T> {
         let p = Packet {
             queue: mpsc::Queue::new(),
@@ -78,11 +79,18 @@ impl<T: Send> Packet<T> {
             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().

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;