From 3b901385d0ac0e0a6e0849f41bb98b8e61229773 Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Tue, 6 Jan 2026 18:27:19 +0800 Subject: [PATCH] [Fix](Exception) Fix potential use-after-free because `Exception::to_string` is not thread safe (#59558) ### What problem does this PR solve? Problem Summary: `Exception::to_string()` may by accessed concurrently in the following situation: - thread A and thread B access the same `DorisCallOnce` object - An Exception `e` is throw when thread A is calling `DorisCallOnce::call` and `e` is thrown to thread A - `e` will be also thrown to thread B later by DorisCallOnce - thread A and thread B access `Exception::to_string()` concurrently This may cause use-after-free due to the assignment of `_cache_string` in `Exception::to_string` Considering that `Exception` should not be frequently used, this PR construct `_cache_string` in constructor `Exception::Exception` rather than lazily creating it. This can avoid additional unnessary synchronazation. ### Release note None ### Check List (For Author) - Test - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason - Behavior changed: - [ ] No. - [ ] Yes. - Does this need documentation? - [ ] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label --- be/src/common/exception.cpp | 8 ++++++++ be/src/common/exception.h | 20 +++----------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/be/src/common/exception.cpp b/be/src/common/exception.cpp index 371a369b7be079..297b07222de364 100644 --- a/be/src/common/exception.cpp +++ b/be/src/common/exception.cpp @@ -48,6 +48,14 @@ Exception::Exception(int code, const std::string_view& msg, bool from_status) { // std::cout << "Exception: " << code << ", " << msg << ", " << get_stack_trace(0, "DISABLED") // << std::endl; #endif + + fmt::memory_buffer buf; + fmt::format_to(buf, "[E{}] {}", _code, _err_msg->_msg); + if (!_err_msg->_stack.empty()) { + fmt::format_to(buf, "\n{}", _err_msg->_stack); + } + _cache_string = fmt::to_string(buf); + if (config::exit_on_exception) { LOG(FATAL) << "[ExitOnException] error code: " << code << ", message: " << msg; } diff --git a/be/src/common/exception.h b/be/src/common/exception.h index c24839eb5416ba..bcf4aea42eeada 100644 --- a/be/src/common/exception.h +++ b/be/src/common/exception.h @@ -46,9 +46,9 @@ class Exception : public std::exception { int code() const { return _code; } std::string message() const { return _err_msg ? _err_msg->_msg : ""; } - const std::string& to_string() const; + const std::string& to_string() const { return _cache_string; } - const char* what() const noexcept override { return to_string().c_str(); } + const char* what() const noexcept override { return _cache_string.c_str(); } Status to_status() const { return {code(), _err_msg->_msg, _err_msg->_stack}; } @@ -61,22 +61,8 @@ class Exception : public std::exception { std::string _stack; }; std::unique_ptr _err_msg; - mutable std::string _cache_string; + std::string _cache_string {}; }; - -inline const std::string& Exception::to_string() const { - if (!_cache_string.empty()) { - return _cache_string; - } - fmt::memory_buffer buf; - fmt::format_to(buf, "[E{}] {}", _code, _err_msg ? _err_msg->_msg : ""); - if (_err_msg && !_err_msg->_stack.empty()) { - fmt::format_to(buf, "\n{}", _err_msg->_stack); - } - _cache_string = fmt::to_string(buf); - return _cache_string; -} - } // namespace doris #define RETURN_IF_CATCH_EXCEPTION(stmt) \