github-actions[bot] commented on code in PR #64077:
URL: https://github.com/apache/doris/pull/64077#discussion_r3351606400
##########
be/src/exec/operator/spill_utils.h:
##########
@@ -78,8 +78,10 @@ struct SpillContext {
// small utility to run the provided callbacks and forward cancellation.
inline Status run_spill_task(RuntimeState* state, std::function<Status()>
exec_func,
std::function<Status()> fin_cb = {}) {
+ RETURN_IF_CANCELLED(state);
RETURN_IF_ERROR(exec_func());
Review Comment:
This pre-callback cancellation check can skip cleanup that current callers
perform inside `exec_func` after they have already changed shared state. For
example, `PartitionedAggSinkLocalState::_revoke_memory()` increments
`task_controller()->increase_revoking_tasks_count()` before calling
`run_spill_task()`, and the matching decrement is in the callback's `Defer`; if
the query is cancelled in that window, this new check returns before the
callback and leaks the revoking task count. Similarly,
`MultiCastDataStreamer::_trigger_spill_if_need()` blocks `_write_dependency`
before `_start_spill_task()`, while `_start_spill_task()` relies on its
callback to call `_write_dependency->set_ready()`. Please keep caller
cleanup/accounting guaranteed, e.g. do not skip `exec_func` after caller setup,
or move such cleanup outside the callback before adding this helper-level early
return.
--
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]
