]> git.lizzy.rs Git - rust.git/commitdiff
Fix undefined behavior when re-locking a mutex from the same thread
authorAmanieu d'Antras <amanieu@gmail.com>
Wed, 25 May 2016 04:44:28 +0000 (05:44 +0100)
committerAmanieu d'Antras <amanieu@gmail.com>
Thu, 2 Jun 2016 12:31:01 +0000 (13:31 +0100)
The only applies to pthread mutexes. We solve this by creating the
mutex with the PTHREAD_MUTEX_NORMAL type, which guarantees that
re-locking from the same thread will deadlock.

src/libstd/sync/mutex.rs
src/libstd/sys/common/mutex.rs
src/libstd/sys/unix/mutex.rs
src/libstd/sys/windows/mutex.rs

index 90cc79dad66bba67d9e82adf096fae276498d4bc..e53a97eccdfca41e179ff3b2f73c1d7e4dab9901 100644 (file)
@@ -190,10 +190,14 @@ impl<T> Mutex<T> {
     /// Creates a new mutex in an unlocked state ready for use.
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn new(t: T) -> Mutex<T> {
-        Mutex {
+        let mut m = Mutex {
             inner: box StaticMutex::new(),
             data: UnsafeCell::new(t),
+        };
+        unsafe {
+            m.inner.lock.init();
         }
+        m
     }
 }
 
index 5a6dfe7fb1a1507739b6b47447490ec60d426b92..7a2183c522f5b92154b198e55aea3cdc4455a5f4 100644 (file)
@@ -27,6 +27,12 @@ impl Mutex {
     /// first used with any of the functions below.
     pub const fn new() -> Mutex { Mutex(imp::Mutex::new()) }
 
+    /// Prepare the mutex for use.
+    ///
+    /// This should be called once the mutex is at a stable memory address.
+    #[inline]
+    pub unsafe fn init(&mut self) { self.0.init() }
+
     /// Locks the mutex blocking the current thread until it is available.
     ///
     /// Behavior is undefined if the mutex has been moved between this and any
index 4e4abcfbeee4ddb9256451e8f6bdb54e9a0f3d4f..52cf3f97c5c83ba49e12fb37a746710d87840160 100644 (file)
@@ -30,6 +30,39 @@ pub const fn new() -> Mutex {
         Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
     }
     #[inline]
+    pub unsafe fn init(&mut self) {
+        // Issue #33770
+        //
+        // A pthread mutex initialized with PTHREAD_MUTEX_INITIALIZER will have
+        // a type of PTHREAD_MUTEX_DEFAULT, which has undefined behavior if you
+        // try to re-lock it from the same thread when you already hold a lock.
+        //
+        // In practice, glibc takes advantage of this undefined behavior to
+        // implement hardware lock elision, which uses hardware transactional
+        // memory to avoid acquiring the lock. While a transaction is in
+        // progress, the lock appears to be unlocked. This isn't a problem for
+        // other threads since the transactional memory will abort if a conflict
+        // is detected, however no abort is generated if re-locking from the
+        // same thread.
+        //
+        // Since locking the same mutex twice will result in two aliasing &mut
+        // references, we instead create the mutex with type
+        // PTHREAD_MUTEX_NORMAL which is guaranteed to deadlock if we try to
+        // re-lock it from the same thread, thus avoiding undefined behavior.
+        //
+        // We can't do anything for StaticMutex, but that type is deprecated
+        // anyways.
+        let mut attr: libc::pthread_mutexattr_t = mem::uninitialized();
+        let r = libc::pthread_mutexattr_init(&mut attr);
+        debug_assert_eq!(r, 0);
+        let r = libc::pthread_mutexattr_settype(&mut attr, libc::PTHREAD_MUTEX_NORMAL);
+        debug_assert_eq!(r, 0);
+        let r = libc::pthread_mutex_init(self.inner.get(), &attr);
+        debug_assert_eq!(r, 0);
+        let r = libc::pthread_mutexattr_destroy(&mut attr);
+        debug_assert_eq!(r, 0);
+    }
+    #[inline]
     pub unsafe fn lock(&self) {
         let r = libc::pthread_mutex_lock(self.inner.get());
         debug_assert_eq!(r, 0);
index b770156582d3bd6ff87ba053d1fd7af9833bc235..8762b34e3da484df7fbf49854f4ac57f754a8a1d 100644 (file)
@@ -64,6 +64,8 @@ pub const fn new() -> Mutex {
             held: UnsafeCell::new(false),
         }
     }
+    #[inline]
+    pub unsafe fn init(&mut self) {}
     pub unsafe fn lock(&self) {
         match kind() {
             Kind::SRWLock => c::AcquireSRWLockExclusive(raw(self)),