From 946f39b4774d11d467c33eb0c43c659108d392eb Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 11 Jan 2024 11:51:40 -0800 Subject: [PATCH] QSemaphore::release: Revert "Optimize cond var notification" This reverts commit 60113056bc4c328f62808d1c0fa2a1abec481f78. Calling d->cond.notify_all(); without the mutex means that another thread could acquire the semaphore (acquire the mutex, subtract from d->avail, return to caller) and destroy it. That would mean this thread is now effectively dereferencing a dangling d pointer. Fixes: QTBUG-120762 Pick-to: 6.6 6.5 Change-Id: I196523f9addf41c2bf1ffffd17a96317f88b43dd Reviewed-by: Marc Mutz Reviewed-by: Artem Dyomin (cherry picked from commit 763ab0e6236de80a0b589fc574c75a414d86d374) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/thread/qsemaphore.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/corelib/thread/qsemaphore.cpp b/src/corelib/thread/qsemaphore.cpp index b11a7051b64..72781942acd 100644 --- a/src/corelib/thread/qsemaphore.cpp +++ b/src/corelib/thread/qsemaphore.cpp @@ -401,10 +401,10 @@ void QSemaphore::release(int n) return; } - { - const auto locker = qt_scoped_lock(d->mutex); - d->avail += n; - } + // Keep mutex locked until after notify_all() lest another thread acquire()s + // the semaphore once d->avail == 0 and then destroys it, leaving `d` dangling. + const auto locker = qt_scoped_lock(d->mutex); + d->avail += n; d->cond.notify_all(); }