Commit e13c8b4
committed
[fix](executor) Fix rare self-deadlock that can cause the time-sharing task executor to hang. (apache#58273)
1. Fix a self-deadlock that can occur in `_dispatch_thread` when a
destructor path attempts to re-acquire the same mutex already held by
the thread. The root cause is destructors (triggered while holding
`_lock`) performing operations that try to re-acquire the same mutex. A
safe fix should ensure destructors that may call `remove_task()` run
outside the `_lock` scope or avoid re-locking the same mutex inside
destructor paths.
2. Call `_task_executor->wait()` in
`TaskExecutorSimplifiedScanScheduler::stop()`.
---
1. `std::shared_ptr<PrioritizedSplitRunner> split =
_tokenless->_entries->take();`
3. `l.lock();` — `_dispatch_thread` acquires `_lock`.
4. After the `while` loop finishes, `split` goes out of scope and the
`shared_ptr` is destroyed.
5. `PrioritizedSplitRunner` destructor runs → destroys `_split_runner`
(`ScannerSplitRunner`).
6. `ScannerSplitRunner::_scan_func` destructor runs → destroys captured
`ctx` (`std::shared_ptr<ScannerContext>`).
7. `ScannerContext::~ScannerContext()` calls `remove_task()`.
8. `remove_task()` attempts to acquire `_lock`.
9. Result: **self-deadlock** because `_lock` is already held by
`_dispatch_thread`.
---
We must explicitly release 'split' BEFORE acquiring _lock to avoid
self-deadlock. The destructor chain (`PrioritizedSplitRunner` ->
`ScannerSplitRunner`-> `_scan_func` lambda -> captured `ScannerContext`)
may call `remove_task()` which tries to acquire `_lock`. Since `_lock`
is not a recursive mutex, this would deadlock.1 parent a6e5d20 commit e13c8b4
File tree
4 files changed
+14
-10
lines changed- be/src/vec/exec
- executor
- time_sharing
- scan
4 files changed
+14
-10
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| 40 | + | |
40 | 41 | | |
41 | 42 | | |
42 | 43 | | |
| |||
Lines changed: 11 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
562 | 562 | | |
563 | 563 | | |
564 | 564 | | |
565 | | - | |
566 | | - | |
567 | | - | |
568 | | - | |
569 | 565 | | |
570 | 566 | | |
571 | 567 | | |
| |||
577 | 573 | | |
578 | 574 | | |
579 | 575 | | |
580 | | - | |
581 | | - | |
582 | | - | |
583 | | - | |
584 | 576 | | |
585 | 577 | | |
586 | 578 | | |
| |||
625 | 617 | | |
626 | 618 | | |
627 | 619 | | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
628 | 631 | | |
629 | 632 | | |
630 | 633 | | |
| |||
Lines changed: 1 addition & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
| 100 | + | |
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
| |||
245 | 246 | | |
246 | 247 | | |
247 | 248 | | |
248 | | - | |
249 | | - | |
250 | 249 | | |
251 | 250 | | |
252 | 251 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
268 | 268 | | |
269 | 269 | | |
270 | 270 | | |
| 271 | + | |
271 | 272 | | |
272 | 273 | | |
273 | 274 | | |
| |||
0 commit comments