github-actions[bot] commented on code in PR #67942:
URL: https://github.com/apache/doris/pull/67942#discussion_r4003779238
##########
be/src/exec/operator/olap_scan_operator.cpp:
##########
@@ -878,16 +878,17 @@ Status
OlapScanLocalState::_sync_cloud_tablets(RuntimeState* state) {
tasks.emplace_back([this, sync_stats, version, i, task_ctx,
task_create_time]() {
// Record bthread scheduling delay
auto task_start_time = std::chrono::steady_clock::now();
+ auto task_lock = task_ctx.lock();
+ if (task_lock == nullptr) {
+ return Status::OK();
+ }
+ // The local state owns sync_stats, so keep its context
alive before access.
if (sync_stats) {
sync_stats->bthread_schedule_delay_ns +=
std::chrono::duration_cast<std::chrono::nanoseconds>(
task_start_time - task_create_time)
.count();
}
- auto task_lock = task_ctx.lock();
- if (task_lock == nullptr) {
- return Status::OK();
- }
Defer defer([&] {
Review Comment:
**[P1] Complete the dependency when fork/join short-circuits**
`cloud::bthread_fork_join` stops dispatching new tasks after the first child
returns non-OK, but `_pending_tablets_num` was initialized to every scan range
and only lambdas that actually enter this `Defer` decrement it. With more
tablets than the concurrency limit, an early `get_tablet`/`sync_rowsets`
failure therefore leaves the count positive and `_cloud_tablet_dependency`
blocked forever; the future contains the error, but `prepare()` cannot reach
`future.get()` while the dependency is blocked. Please drive readiness from
fork/join completion or otherwise account for tasks the helper skips, and cover
the early-failure case with more tasks than the concurrency limit.
##########
be/src/exec/operator/olap_scan_operator.cpp:
##########
@@ -878,16 +878,17 @@ Status
OlapScanLocalState::_sync_cloud_tablets(RuntimeState* state) {
tasks.emplace_back([this, sync_stats, version, i, task_ctx,
task_create_time]() {
// Record bthread scheduling delay
auto task_start_time = std::chrono::steady_clock::now();
+ auto task_lock = task_ctx.lock();
Review Comment:
**[P1] Apply this ordering to the parallel async-writer path**
The same cancellation/UAF ordering still exists in
`be/src/exec/sink/writer/async_result_writer.cpp:124-125`: the queued worker
calls `SCOPED_ATTACH_TASK(state)` before `task_ctx.lock()`, and
`AttachTask(RuntimeState*)` immediately dereferences that raw state.
Cancellation can mark finish dependencies always-ready and destroy the
PFC-owned state while the pool job is still queued, reproducing the crash class
fixed here. Please acquire and validate the context lock before
`SCOPED_ATTACH_TASK(state)` and retain it through `process_block()`.
--
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]
