logger_config_guard's destructor resets the logger unconditionally and without holding g_guard_mutex (cpp/src/utilities/logger.cpp:141-142, and reset_default_logger at :127-130):
struct logger_config_guard {
~logger_config_guard() { cuopt::reset_default_logger(); }
};Two problems, both on main today.
A stale guard can reset a newer configuration. A guard's refcount reaching zero expires g_active_guard before the destructor body runs. In that window another thread calling init_logger_t sees no live configuration, applies its own and installs a new guard. The first destructor then runs and resets the logger out from under it, so that thread's messages go to the buffer sink and are dropped while it still holds a live handle.
Unsynchronized sink mutation. reset_default_logger does sinks().clear() / push_back() with no lock, while init_logger_t's configure path mutates the same vector under g_guard_mutex (:151, :160-168). Two threads can mutate it concurrently.
Reproducing
Needs the refcount to hit zero exactly while another thread is inside the configure path. Under contention g_active_guard.lock() usually succeeds instead, so the window is rarely entered and a straightforward stress test does not reliably hit it. A TSAN build over concurrent solves that each construct an init_logger_t is the more likely way to surface the sink mutation.
Suggested fix
Give each configuration a generation number, record it in the guard, and have the destructor take g_guard_mutex and reset only if its own generation is still current. That closes the stale-reset window and puts sink mutation on the configure and reset paths under the same lock.
Exposure is limited in practice: configuration happens at solve boundaries rather than concurrently with a solve that is emitting. Split out of #1778, which found it but where the fix was out of scope.
logger_config_guard's destructor resets the logger unconditionally and without holdingg_guard_mutex(cpp/src/utilities/logger.cpp:141-142, andreset_default_loggerat:127-130):Two problems, both on
maintoday.A stale guard can reset a newer configuration. A guard's refcount reaching zero expires
g_active_guardbefore the destructor body runs. In that window another thread callinginit_logger_tsees no live configuration, applies its own and installs a new guard. The first destructor then runs and resets the logger out from under it, so that thread's messages go to the buffer sink and are dropped while it still holds a live handle.Unsynchronized sink mutation.
reset_default_loggerdoessinks().clear()/push_back()with no lock, whileinit_logger_t's configure path mutates the same vector underg_guard_mutex(:151,:160-168). Two threads can mutate it concurrently.Reproducing
Needs the refcount to hit zero exactly while another thread is inside the configure path. Under contention
g_active_guard.lock()usually succeeds instead, so the window is rarely entered and a straightforward stress test does not reliably hit it. A TSAN build over concurrent solves that each construct aninit_logger_tis the more likely way to surface the sink mutation.Suggested fix
Give each configuration a generation number, record it in the guard, and have the destructor take
g_guard_mutexand reset only if its own generation is still current. That closes the stale-reset window and puts sink mutation on the configure and reset paths under the same lock.Exposure is limited in practice: configuration happens at solve boundaries rather than concurrently with a solve that is emitting. Split out of #1778, which found it but where the fix was out of scope.