jacktengg opened a new pull request, #65827:
URL: https://github.com/apache/doris/pull/65827
### What problem does this PR solve?
Issue Number: None
Related PR: #51596
Problem Summary: BlockingQueue split ownership of waiter counters between
waiting threads and notifying threads. Around a timeout race, both sides could
decrement the same registration, while a spurious wakeup could leave a stale
registration behind. Once the counters drifted, a real waiter could be
represented by zero and miss a notification, leaving routine load queue
operations blocked until the one-hour fallback timeout. This is a missed wakeup
and potentially long stall, rather than a permanent deadlock. In addition,
try_put read the shutdown flag and queue size before acquiring the mutex, which
introduced a data race.
The following sequence shows how lock contention after a timeout corrupts
the consumer waiter count:
| Time | Consumer threads | Producer threads | get_waiting | Result |
| --- | --- | --- | ---: | --- |
| T0 | C increments the counter and calls wait_for, which releases the
mutex. | - | 0 -> 1 | C is registered as a condition-variable waiter. |
| T1 | C's timeout expires. C leaves the condition-variable wait set, but
wait_for must reacquire the mutex before returning. | P acquires the mutex
before C can reacquire it. | 1 | C's registration is still visible because C
cannot update it without the mutex. |
| T2 | C remains blocked while trying to reacquire the mutex. | P pushes an
item, decrements the counter from 1 to 0, unlocks, and calls notify_one. | 1 ->
0 | The notification cannot wake C because its timed wait has already expired. |
| T3 | C reacquires the mutex. wait_for returns timeout, so the old timeout
branch decrements the same registration again. C then consumes the item. | - |
0 -> SIZE_MAX | The unsigned waiter count wraps around. |
| T4 | After the queue becomes empty, a new consumer C2 increments the
counter and starts waiting. | - | SIZE_MAX -> 0 | A real waiter is now
represented by zero. |
| T5 | Absent a spurious wakeup, shutdown, or another notification, C2
remains asleep until its next timeout, which is one hour by default. | P2
pushes an item, observes zero, and skips notify_one. | 0 | C2 suffers a missed
wakeup and a potentially one-hour stall. |
Make each waiting thread register immediately before wait_for and always
unregister after wait_for returns, regardless of the wakeup reason. Notifiers
now inspect waiter counters under the mutex without consuming registrations,
then release the mutex before notifying. Remove the unsynchronized try_put fast
path and add deterministic tests for both consumer and producer timeout races.
### Release note
Fix potential long stalls in backend blocking queues under concurrent
timeout and notification.
### Check List (For Author)
- Test: Unit Test
- Added BlockingQueueWaiterTest.TimedGetNotificationRace and
BlockingQueueWaiterTest.TimedPutNotificationRace.
- `./run-be-ut.sh --run --filter='BlockingQueueWaiterTest.*' -j 32`
- Behavior changed: Yes. Waiter registrations are now released by the
waiting thread, and try_put checks queue state only while holding the mutex.
- Does this need documentation: No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
