]> git.lizzy.rs Git - rust.git/commitdiff
Don't allow pthread_rwlock_t to recursively lock itself
authorAmanieu d'Antras <amanieu@gmail.com>
Wed, 25 May 2016 05:07:54 +0000 (06:07 +0100)
committerAmanieu d'Antras <amanieu@gmail.com>
Thu, 2 Jun 2016 12:31:01 +0000 (13:31 +0100)
This is allowed by POSIX and can happen on glibc with processors
that support hardware lock elision.

src/libstd/sys/unix/rwlock.rs

index 44bd5d895f2e4373af64e753349c799b193be556..72ab70aeac482515370ed5e96c396c7466d52479 100644 (file)
 use libc;
 use cell::UnsafeCell;
 
-pub struct RWLock { inner: UnsafeCell<libc::pthread_rwlock_t> }
+pub struct RWLock {
+    inner: UnsafeCell<libc::pthread_rwlock_t>,
+    write_locked: UnsafeCell<bool>,
+}
 
 unsafe impl Send for RWLock {}
 unsafe impl Sync for RWLock {}
 
 impl RWLock {
     pub const fn new() -> RWLock {
-        RWLock { inner: UnsafeCell::new(libc::PTHREAD_RWLOCK_INITIALIZER) }
+        RWLock {
+            inner: UnsafeCell::new(libc::PTHREAD_RWLOCK_INITIALIZER),
+            write_locked: UnsafeCell::new(false),
+        }
     }
     #[inline]
     pub unsafe fn read(&self) {
@@ -35,7 +41,16 @@ pub unsafe fn read(&self) {
         //
         // We roughly maintain the deadlocking behavior by panicking to ensure
         // that this lock acquisition does not succeed.
-        if r == libc::EDEADLK {
+        //
+        // We also check whether there this lock is already write locked. This
+        // is only possible if it was write locked by the current thread and
+        // the implementation allows recursive locking. The POSIX standard
+        // doesn't require recursivly locking a rwlock to deadlock, but we can't
+        // allow that because it could lead to aliasing issues.
+        if r == libc::EDEADLK || *self.write_locked.get() {
+            if r == 0 {
+                self.raw_unlock();
+            }
             panic!("rwlock read lock would result in deadlock");
         } else {
             debug_assert_eq!(r, 0);
@@ -43,29 +58,57 @@ pub unsafe fn read(&self) {
     }
     #[inline]
     pub unsafe fn try_read(&self) -> bool {
-        libc::pthread_rwlock_tryrdlock(self.inner.get()) == 0
+        let r = libc::pthread_rwlock_tryrdlock(self.inner.get());
+        if r == 0 && *self.write_locked.get() {
+            self.raw_unlock();
+            false
+        } else {
+            r == 0
+        }
     }
     #[inline]
     pub unsafe fn write(&self) {
         let r = libc::pthread_rwlock_wrlock(self.inner.get());
-        // see comments above for why we check for EDEADLK
-        if r == libc::EDEADLK {
+        // see comments above for why we check for EDEADLK and write_locked
+        if r == libc::EDEADLK || *self.write_locked.get() {
+            if r == 0 {
+                self.raw_unlock();
+            }
             panic!("rwlock write lock would result in deadlock");
         } else {
             debug_assert_eq!(r, 0);
         }
+        *self.write_locked.get() = true;
     }
     #[inline]
     pub unsafe fn try_write(&self) -> bool {
-        libc::pthread_rwlock_trywrlock(self.inner.get()) == 0
+        let r = libc::pthread_rwlock_trywrlock(self.inner.get());
+        if r == 0 && *self.write_locked.get() {
+            self.raw_unlock();
+            false
+        } else if r == 0 {
+            *self.write_locked.get() = true;
+            true
+        } else {
+            false
+        }
     }
     #[inline]
-    pub unsafe fn read_unlock(&self) {
+    unsafe fn raw_unlock(&self) {
         let r = libc::pthread_rwlock_unlock(self.inner.get());
         debug_assert_eq!(r, 0);
     }
     #[inline]
-    pub unsafe fn write_unlock(&self) { self.read_unlock() }
+    pub unsafe fn read_unlock(&self) {
+        debug_assert!(!*self.write_locked.get());
+        self.raw_unlock();
+    }
+    #[inline]
+    pub unsafe fn write_unlock(&self) {
+        debug_assert!(*self.write_locked.get());
+        *self.write_locked.get() = false;
+        self.raw_unlock();
+    }
     #[inline]
     pub unsafe fn destroy(&self) {
         let r = libc::pthread_rwlock_destroy(self.inner.get());