dlerror: don't gratuitously hold freebuf_queue lock while freeing

thread-local buffers allocated for dlerror need to be queued for free
at a later time when the owning thread exits, since malloc may be
replaced by application code and the exiting context is not valid to
call application code from. the code to process queue of pending
frees, introduced in commit aa5a9d15e09851f7b4a1668e9dbde0f6234abada,
gratuitously held the lock for the entire duration of queue
processing, updating the global queue pointer after each free, despite
there being no logical requirement that all frees finish before
another thread can access the queue.

instead, immediately claim the whole queue for freeing and release the
lock, then walk the list and perform frees without the lock held. the
change is unlikely to make any meaningful difference to performance,
but it eliminates one point where the allocator is called under an
internal lock. since the allocator may be application-provided, such
calls are undesirable because they allow application code to impede
forward progress of libc functions in other threads arbitrarily long,
and to induce deadlock if it calls a libc function that requires the
same lock.

the change also eliminates a lock ordering consideration that's an
impediment upcoming work with multithreaded fork.

1 files changed, 8 insertions, 5 deletions

diff --git a/src/ldso/dlerror.c b/src/ldso/dlerror.c
index 3fcc7779..d8bbfc03 100644
--- a/src/ldso/dlerror.c
+++ b/src/ldso/dlerror.c
@@ -35,13 +35,16 @@ void __dl_thread_cleanup(void)
 hidden void __dl_vseterr(const char *fmt, va_list ap)
 {
 	LOCK(freebuf_queue_lock);
-	while (freebuf_queue) {
-		void **p = freebuf_queue;
-		freebuf_queue = *p;
-		free(p);
-	}
+	void **q = freebuf_queue;
+	freebuf_queue = 0;
 	UNLOCK(freebuf_queue_lock);
 
+	while (q) {
+		void **p = *q;
+		free(q);
+		q = p;
+	}
+
 	va_list ap2;
 	va_copy(ap2, ap);
 	pthread_t self = __pthread_self();