branch-4.1: [fix](rpc) Fix AutoReleaseClosure data race with callback reuse (#61782) - #67340
Open
linrrzqqq wants to merge 1 commit into
Open
branch-4.1: [fix](rpc) Fix AutoReleaseClosure data race with callback reuse (#61782)#67340linrrzqqq wants to merge 1 commit into
linrrzqqq wants to merge 1 commit into
Conversation
…he#61782) The callback's call() method may reuse the callback object (e.g., in vdata_stream_sender.h get_send_callback()), triggering a new RPC that mutates response_ and cntl_. If AutoReleaseClosure::Run() invokes call() before checking cntl_->Failed() or response_->status(), it reads the NEW RPC's state instead of the ORIGINAL RPC's result, causing: ``` *** SIGSEGV address not mapped to object (@0x0) received by PID 238162 (TID 240463 OR 0xfffa2c9898e0) from PID 0; stack trace: *** 0# doris::signal::(anonymous namespace)::FailureSignalHandler(int, siginfo_t*, void*) at /home/zcp/repo_center/doris_release/doris/be/src/common/signal_handler.h:421 1# os::Linux::chained_handler(int, siginfo_t*, void*) in /opt/module/doris/java8/jre/lib/aarch64/server/libjvm.so 2# JVM_handle_linux_signal in /opt/module/doris/java8/jre/lib/aarch64/server/libjvm.so 3# signalHandler(int, siginfo_t*, void*) in /opt/module/doris/java8/jre/lib/aarch64/server/libjvm.so 4# 0x0000FFFF0AB107C0 in linux-vdso.so.1 5# doris::Status doris::Status::create<true>(doris::PStatus const&) at /home/zcp/repo_center/doris_release/doris/be/src/common/status.h:398 6# void doris::AutoReleaseClosure<doris::PTransmitDataParams, doris::pipeline::ExchangeSendCallback<doris::PTransmitDataResult> >::_process_status<doris::PTransmitDataResult>(doris::PTransmitDataResult*) at /home/zcp/repo_center/doris_release/doris/be/src/util/ref_count_closure.h:128 7# doris::AutoReleaseClosure<doris::PTransmitDataParams, doris::pipeline::ExchangeSendCallback<doris::PTransmitDataResult> >::Run() at /home/zcp/repo_center/doris_release/doris/be/src/util/ref_count_closure.h:102 8# brpc::Controller::EndRPC(brpc::Controller::CompletionInfo const&) in /opt/module/doris/be/lib/doris_be 9# brpc::policy::ProcessRpcResponse(brpc::InputMessageBase*) in /opt/module/doris/be/lib/doris_be 10# brpc::ProcessInputMessage(void*) in /opt/module/doris/be/lib/doris_be 11# bthread::TaskGroup::task_runner(long) in /opt/module/doris/be/lib/doris_be 12# bthread_make_fcontext in /opt/module/doris/be/lib/doris_be ``` we have confirmed the data race is real existing with temporary LOGs which has been removed: ``` F20260325 21:46:58.465230 3453395 brpc_closure.h:116] Check failed: _debug_generation_at_construction == current_gen (2 vs. 3) RACE DETECTED: AutoReleaseClosure response_ was reused by a new RPC (generation changed from 2 to 3) while still in Run(). The old closure is about to read response_->status() but the new RPC may be concurrently writing to the same response_ object. ``` and we add some be-ut which could only pass WITH this patch. before we fix: ``` [----------] 7 tests from ExchangeSinkTest (5 ms total) [----------] Global test environment tear-down [==========] 7 tests from 1 test suite ran. (5 ms total) [ PASSED ] 4 tests. [ FAILED ] 3 tests, listed below: [ FAILED ] ExchangeSinkTest.test_closure_call_must_not_corrupt_status_check [ FAILED ] ExchangeSinkTest.test_closure_call_must_not_hide_error_status [ FAILED ] ExchangeSinkTest.test_closure_call_must_not_hide_rpc_failure ``` after: ``` [----------] 7 tests from ExchangeSinkTest (4 ms total) [----------] Global test environment tear-down [==========] 7 tests from 1 test suite ran. (5 ms total) [ PASSED ] 7 tests. ```
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Collaborator
Author
|
run buildall |
16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pick: #61782