github-actions[bot] commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3020501515


##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -474,6 +474,15 @@ Status PipelineTask::execute(bool* done) {
 
         // If task is woke up early, we should terminate all operators, and 
this task could be closed immediately.
         if (_wake_up_early) {
+            // If the task is in BLOCKED state (e.g., _is_blocked() returned 
true right before
+            // another thread called make_all_runnable), we must transition 
back to RUNNABLE first.
+            // This ensures close() sees a legal RUNNABLE→FINISHED transition, 
and also races with
+            // Dependency::set_ready()'s delayed wake_up() call: wake_up() 
will see the task is
+            // already RUNNABLE and be handled safely (see wake_up() below).
+            if (_exec_state == State::BLOCKED) {
+                _blocked_dep = nullptr;

Review Comment:
   `execute()` now clears `_blocked_dep` and flips `_exec_state` back to 
`RUNNABLE` without taking the dependency's `_task_lock`, but 
`Dependency::set_ready()` still invokes `wake_up()` while holding that lock. A 
concrete interleaving is: (1) the task is blocked on dependency `D`, (2) 
`set_ready()` swaps `_blocked_task` and enters `wake_up()`, (3) the worker 
thread executes this block and stores `_blocked_dep = nullptr`, and then (4) 
the readiness thread still continues down the old `BLOCKED` path and reads 
`_blocked_dep` in `DCHECK_EQ`. That is a data race on a plain pointer, and in 
debug/ASAN builds it can still trip the DCHECK. I think this transition needs 
to stay synchronized with the existing dependency lock protocol instead of 
mutating `_blocked_dep` here unsafely.



-- 
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]

Reply via email to