From 587e3bb0ba6def90906f1f6b8c60c8f33ec88571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 25 Mar 2021 17:26:37 +0100 Subject: [PATCH] QSemaphore: fix missed wakes with futex on Windows We do this by making the 'futexNeedsWakeAllBit' to be part of both the expectedValue and the pointer we're waiting on. This makes sense since having the 'futexNeedsWakeAllBit' set is a requirement for starting a sleep. Thus we should condition sleeping on whether or not another thread unset it. Since the futexNeedsWakeAllBit is in the "topmost" bit of the pointer we wait on we'll need to use the full 64-bits on 64-bit platforms. This isn't enabled (nor was it an issue) for configurations with 'futexHasWaiterCount' since that works differently, and waits for the high 32 bits. Fixes: QTBUG-92148 Change-Id: I424c605f0120ea5e647c5bb19b00ff35eaf1608a Reviewed-by: Thiago Macieira --- src/corelib/thread/qsemaphore.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/corelib/thread/qsemaphore.cpp b/src/corelib/thread/qsemaphore.cpp index 5111d80ac6b..45983d47ab1 100644 --- a/src/corelib/thread/qsemaphore.cpp +++ b/src/corelib/thread/qsemaphore.cpp @@ -200,14 +200,21 @@ futexSemaphoreTryAcquire_loop(QBasicAtomicInteger &u, quintptr curValu // indicate we're waiting start_wait: - auto ptr = futexLow32(&u); + auto ptr = [&u]() { + if constexpr (futexHasWaiterCount) + return futexLow32(&u); + else + return &u; + }(); if (n > 1 || !futexHasWaiterCount) { u.fetchAndOrRelaxed(futexNeedsWakeAllBit); curValue |= futexNeedsWakeAllBit; - if (n > 1 && futexHasWaiterCount) { - ptr = futexHigh32(&u); - //curValue >>= 32; // but this is UB in 32-bit, so roundabout: - curValue = quint64(curValue) >> 32; + if constexpr (futexHasWaiterCount) { + if (n > 1) { + ptr = futexHigh32(&u); + // curValue >>= 32; // but this is UB in 32-bit, so roundabout: + curValue = quint64(curValue) >> 32; + } } } @@ -397,7 +404,7 @@ void QSemaphore::release(int n) futexWakeOp(*futexLow32(&u), n, INT_MAX, *futexHigh32(&u), FUTEX_OP(op, oparg, cmp, cmparg)); } #else - // Unset the bit and wake everyone. There are two possibibilies + // Unset the bit and wake everyone. There are two possibilities // under which a thread can set the bit between the AND and the // futexWake: // 1) it did see the new counter value, but it wasn't enough for