]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
common: do not unlock rwlock on destruction 3963/head
authorFederico Simoncelli <fsimonce@redhat.com>
Sat, 15 Nov 2014 14:14:04 +0000 (14:14 +0000)
committerLoic Dachary <ldachary@redhat.com>
Wed, 11 Mar 2015 11:25:35 +0000 (12:25 +0100)
According to pthread_rwlock_unlock(3p):

 Results are undefined if the read-write lock rwlock is not held
 by the calling thread.

and:

 https://sourceware.org/bugzilla/show_bug.cgi?id=17561

 Calling pthread_rwlock_unlock on an rwlock which is not locked
 is undefined.

calling pthread_rwlock_unlock on RWLock destruction could cause
an unknown behavior for two reasons:

- the lock is acquired by another thread (undefined)
- the lock is not acquired (undefined)

Moreover since glibc-2.20 calling pthread_rwlock_unlock on a
rwlock that is not locked results in a SIGILL that kills the
application.

This patch removes the pthread_rwlock_unlock call on destruction
and replaces it with an assertion to check that the RWLock is
not in use.

Any code that relied on the implicit release is now going to
break the assertion, e.g.:

 {
   RWLock l;
   l.get(for_write);
 } // implicit release, wrong.

Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
(cherry picked from commit cf2104d4d991361c53f6e2fea93b69de10cd654b)

src/common/RWLock.h

index 3d94969e5b8be014ba2339753e8ca64931cb7687..1a70ef1d2b13ead328fcd39cd4dfd35da85580fd 100644 (file)
@@ -45,7 +45,9 @@ public:
     return (nwlock.read() > 0);
   }
   virtual ~RWLock() {
-    pthread_rwlock_unlock(&L);
+    // The following check is racy but we are about to destroy
+    // the object and we assume that there are no other users.
+    assert(!is_locked());
     pthread_rwlock_destroy(&L);
   }