fix potential deadlock bug in libc-internal locking logic

if a multithreaded program became non-multithreaded (i.e. all other
threads exited) while one thread held an internal lock, the remaining
thread would fail to release the lock. the the program then became
multithreaded again at a later time, any further attempts to obtain
the lock would deadlock permanently.

the underlying cause is that the value of libc.threads_minus_1 at
unlock time might not match the value at lock time. one solution would
be returning a flag to the caller indicating whether the lock was
taken and needs to be unlocked, but there is a simpler solution: using
the lock itself as such a flag.

note that this flag is not needed anyway for correctness; if the lock
is not held, the unlock code is harmless. however, the memory
synchronization properties associated with a_store are costly on some
archs, so it's best to avoid executing the unlock code when it is
unnecessary.

diff --git a/src/internal/libc.h b/src/internal/libc.h
index 3350b3d1f064420482fce4dd60ad9a3806760d61..d625b56afc3ab310ac644929ff57d03e9a0ca9f1 100644 (file)
--- a/src/internal/libc.h
+++ b/src/internal/libc.h
@@ -53,8 +53,8 @@ void __lock(volatile int *) ATTR_LIBC_VISIBILITY;
 void __unlock(volatile int *) ATTR_LIBC_VISIBILITY;
 int __lockfile(FILE *) ATTR_LIBC_VISIBILITY;
 void __unlockfile(FILE *) ATTR_LIBC_VISIBILITY;
-#define LOCK(x) (libc.threads_minus_1 ? (__lock(x),1) : ((void)(x),1))
-#define UNLOCK(x) (libc.threads_minus_1 ? (__unlock(x),1) : ((void)(x),1))
+#define LOCK(x) __lock(x)
+#define UNLOCK(x) __unlock(x)
 
 void __synccall(void (*)(void *), void *);
 int __setxid(int, int, int, int);

diff --git a/src/malloc/malloc.c b/src/malloc/malloc.c
index 4044eb2af921ffee684e70c2cb849a06af72e454..fb65ab5bb180416166c94c8eca84d6567507dd4f 100644 (file)
--- a/src/malloc/malloc.c
+++ b/src/malloc/malloc.c
@@ -64,28 +64,27 @@ static struct {
 
 static inline void lock(volatile int *lk)
 {
-       if (!libc.threads_minus_1) return;
-       while(a_swap(lk, 1)) __wait(lk, lk+1, 1, 1);
+       if (libc.threads_minus_1)
+               while(a_swap(lk, 1)) __wait(lk, lk+1, 1, 1);
 }
 
 static inline void unlock(volatile int *lk)
 {
-       if (!libc.threads_minus_1) return;
-       a_store(lk, 0);
-       if (lk[1]) __wake(lk, 1, 1);
+       if (lk[0]) {
+               a_store(lk, 0);
+               if (lk[1]) __wake(lk, 1, 1);
+       }
 }
 
 static inline void lock_bin(int i)
 {
-       if (libc.threads_minus_1)
-               lock(mal.bins[i].lock);
+       lock(mal.bins[i].lock);
        if (!mal.bins[i].head)
                mal.bins[i].head = mal.bins[i].tail = BIN_TO_CHUNK(i);
 }
 
 static inline void unlock_bin(int i)
 {
-       if (!libc.threads_minus_1) return;
        unlock(mal.bins[i].lock);
 }
 

diff --git a/src/thread/__lock.c b/src/thread/__lock.c
index 2f345ae7af02153c1bffbd5cbfc0f0be860127a6..0874c04a4c0a81a9fa0dc032479ec18cec81d8aa 100644 (file)
--- a/src/thread/__lock.c
+++ b/src/thread/__lock.c
@@ -2,11 +2,14 @@
 
 void __lock(volatile int *l)
 {
-       while (a_swap(l, 1)) __wait(l, l+1, 1, 1);
+       if (libc.threads_minus_1)
+               while (a_swap(l, 1)) __wait(l, l+1, 1, 1);
 }
 
 void __unlock(volatile int *l)
 {
-       a_store(l, 0);
-       if (l[1]) __wake(l, 1, 1);
+       if (l[0]) {
+               a_store(l, 0);
+               if (l[1]) __wake(l, 1, 1);
+       }
 }