Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions onnxruntime/core/session/inference_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,12 @@ class InferenceSession {
/// convenience pointer to logger. should always be the same as session_state_.Logger();
const logging::Logger* session_logger_;

/// Logger for this session. WARNING: Will contain nullptr if logging_manager_ is nullptr.
/// This MUST be declared before execution_providers_ so the logger outlives EPs during destruction
/// (C++ destroys members in reverse declaration order), allowing EP teardown callbacks to safely
/// use the logger pointer.
std::unique_ptr<logging::Logger> owned_session_logger_ = nullptr;

Comment thread
tianleiwu marked this conversation as resolved.
// The list of execution providers.
// This MUST be prior to model_ in case there are values in the model that were allocated using an allocator
// provided by the EP. If that is the case the allocator's `free` implementation may depend on other parts of the
Expand Down Expand Up @@ -901,9 +907,6 @@ class InferenceSession {
/// User specified logging mgr; logging_manager_ is simply the ptr in this unique_ptr when available
std::unique_ptr<logging::LoggingManager> user_logging_manager_;

/// Logger for this session. WARNING: Will contain nullptr if logging_manager_ is nullptr.
std::unique_ptr<logging::Logger> owned_session_logger_ = nullptr;

// Profiler for this session.
profiling::Profiler session_profiler_;

Expand Down
118 changes: 118 additions & 0 deletions onnxruntime/test/framework/inference_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3041,5 +3041,123 @@ TEST(DeviceStreamCollection, TestOverride) {

#endif // ORT_ENABLE_STREAM

// Test that the session logger outlives execution providers during session destruction.
// This validates the fix for the use-after-free bug where owned_session_logger_ was declared
// after execution_providers_ in InferenceSession, causing the logger to be destroyed before
// EP teardown callbacks could safely use it. See https://github.com/microsoft/onnxruntime/issues/28234.

// An EP that logs via its stored logger pointer during destruction.
// If the logger has been destroyed before the EP, dereferencing the logger is undefined behavior.
class LoggingOnDestroyExecutionProvider : public IExecutionProvider {
public:
static constexpr const char* kEpType = "LoggingOnDestroyExecutionProvider";

// logger_was_valid_in_dtor will be set to true if the logger pointer was non-null
// when the destructor ran.
explicit LoggingOnDestroyExecutionProvider(bool* logger_was_valid_in_dtor,
const std::string& type = kEpType)
: IExecutionProvider{type},
logger_was_valid_in_dtor_(logger_was_valid_in_dtor) {
}

~LoggingOnDestroyExecutionProvider() override {
const logging::Logger* logger = GetLogger();
if (logger != nullptr) {
// Actually use the logger to ensure the pointer is valid (not use-after-free).
// Under AddressSanitizer or similar tools, this would detect a dangling pointer.
LOGS(*logger, VERBOSE) << "LoggingOnDestroyExecutionProvider teardown";
*logger_was_valid_in_dtor_ = true;
} else {
*logger_was_valid_in_dtor_ = false;
}
}

std::shared_ptr<KernelRegistry> GetKernelRegistry() const override {
return std::make_shared<KernelRegistry>();
}

private:
bool* logger_was_valid_in_dtor_;
};

TEST(InferenceSessionTests, SessionLoggerOutlivesEPsOnDestruction) {
Comment thread
tianleiwu marked this conversation as resolved.
// Create a logging manager with VERBOSE level so the logger is non-null and active.
auto capturing_sink = new CapturingSink();
auto logging_manager = std::make_unique<logging::LoggingManager>(
std::unique_ptr<ISink>(capturing_sink), logging::Severity::kVERBOSE, false,
LoggingManager::InstanceType::Temporal);

Comment thread
tianleiwu marked this conversation as resolved.
std::unique_ptr<Environment> env;
OrtThreadingOptions tp_options;
ASSERT_STATUS_OK(Environment::Create(std::move(logging_manager), env, &tp_options,
true /*create_global_thread_pools*/));

bool logger_was_valid_in_dtor = false;

{
SessionOptions so;
so.session_logid = "SessionLoggerOutlivesEPs";
so.session_log_severity_level = static_cast<int>(logging::Severity::kVERBOSE);

InferenceSession session{so, *env};
ASSERT_STATUS_OK(session.RegisterExecutionProvider(
std::make_unique<LoggingOnDestroyExecutionProvider>(&logger_was_valid_in_dtor)));
ASSERT_STATUS_OK(session.Load(MODEL_URI));
ASSERT_STATUS_OK(session.Initialize());

// Session goes out of scope here. Destruction order must be:
// 1. execution_providers_ destroyed (EP teardown logs via logger — must still be valid)
// 2. owned_session_logger_ destroyed (logger freed after all users are done)
}

// The EP's destructor should have found a valid logger and logged successfully.
ASSERT_TRUE(logger_was_valid_in_dtor);

// Verify the EP's teardown log message was actually captured.
auto& msgs = capturing_sink->Messages();
bool found_teardown_msg = std::any_of(msgs.begin(), msgs.end(), [](const std::string& msg) {
return msg.find("LoggingOnDestroyExecutionProvider teardown") != std::string::npos;
});
ASSERT_TRUE(found_teardown_msg)
<< "Expected EP teardown log message not found. Logger may have been destroyed before EP.";
}

TEST(InferenceSessionTests, SessionLoggerOutlivesEPsWithMultipleEPs) {
// Test with multiple EPs to ensure all EPs can log during teardown.
auto capturing_sink = new CapturingSink();
auto logging_manager = std::make_unique<logging::LoggingManager>(
std::unique_ptr<ISink>(capturing_sink), logging::Severity::kVERBOSE, false,
LoggingManager::InstanceType::Temporal);

std::unique_ptr<Environment> env;
OrtThreadingOptions tp_options;
ASSERT_STATUS_OK(Environment::Create(std::move(logging_manager), env, &tp_options,
true /*create_global_thread_pools*/));

bool logger_valid_ep1 = false;
bool logger_valid_ep2 = false;

{
SessionOptions so;
so.session_logid = "SessionLoggerMultipleEPs";
so.session_log_severity_level = static_cast<int>(logging::Severity::kVERBOSE);

InferenceSession session{so, *env};

// Register two EPs with different type names (EP registry requires unique types).
// Both should be able to log safely during teardown.
ASSERT_STATUS_OK(session.RegisterExecutionProvider(
std::make_unique<LoggingOnDestroyExecutionProvider>(&logger_valid_ep1, "LoggingOnDestroyEP_1")));
ASSERT_STATUS_OK(session.RegisterExecutionProvider(
std::make_unique<LoggingOnDestroyExecutionProvider>(&logger_valid_ep2, "LoggingOnDestroyEP_2")));

ASSERT_STATUS_OK(session.Load(MODEL_URI));
ASSERT_STATUS_OK(session.Initialize());
}

ASSERT_TRUE(logger_valid_ep1);
ASSERT_TRUE(logger_valid_ep2);
}

} // namespace test
} // namespace onnxruntime
Loading