github-actions[bot] commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3019842635
##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -1063,7 +1063,12 @@ Status PipelineTask::_state_transition(State new_state) {
}
_task_profile->add_info_string("TaskState", _to_string(new_state));
_task_profile->add_info_string("BlockedByDependency", _blocked_dep ?
_blocked_dep->name() : "");
- if (!LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state)) {
+ bool legal = LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state);
+ // When _wake_up_early is true, a BLOCKED task can skip RUNNABLE and go
directly to FINISHED.
+ if (!legal && _wake_up_early && _exec_state == State::BLOCKED && new_state
== State::FINISHED) {
Review Comment:
This new exception looks unsafe because it legalizes `BLOCKED -> FINISHED`
without also making the dependency wake-up path tolerate an already-closed task.
A concrete race is:
1. The worker thread calls `_is_blocked()` / `_wait_to_start()`, so
`Dependency::is_blocked_by()` stores the task in `_blocked_task` and sets
`_exec_state = BLOCKED`.
2. Another thread calls `set_wake_up_early()` +
`unblock_all_dependencies()`. `Dependency::set_ready()` swaps `_blocked_task`
into a local list, but has not necessarily called `t->wake_up()` yet.
3. The worker thread returns from `execute()`, sees `_wake_up_early`, sets
`done = true`, and `close_task()` now closes/finalizes the task while its state
is still `BLOCKED`.
4. The delayed `wake_up()` still runs from `Dependency::set_ready()` and
tries to transition the same task back to `RUNNABLE` and resubmit it.
Before this patch, the illegal `BLOCKED -> FINISHED` transition exposed that
race immediately. After this patch, the race is only hidden until the later
wake-up path. I think the fix needs to coordinate `close()` with pending
dependency wake-ups (or make `wake_up()` explicitly ignore finished/finalized
tasks), rather than broadening the legal transition table here.
--
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]
