CNA-GAP-002: CNA::Logger serialises writes but not level changes, and a sink must not call back into the logger

CNA snapshot 009d40f5  ·  Known Issues › Functional gaps  ·  source links pinned to 009d40f5

minimumLevel_ is read and written without synchronisation and the sink runs under a non-recursive mutex, so changing the level while other threads log is a data race and a sink that logs deadlocks; the header states neither limit.

Identifier

CNA-GAP-002

Category

Functional gap

Subsystem

Core & runtime

Status

Open

Verified against

CNA 009d40f5 (009d40f5dd085c4e674d3479675fac84b12b3e0a)

Evidence basis

Source-verified: read at TARGET, not executed

Tests touching this area

Yes: see Current tests

Affected contract

CNA::Logger::SetMinimumLevel, GetMinimumLevel, Log and its helpers, SetSink and ResetSink

Expected behaviour

The header documents no thread-safety or re-entrancy rule. The implementation's own comment says the mutex exists so "two threads cannot interleave halves of a line", which invites concurrent use.

Actual behaviour at TARGET

• Logger::minimumLevel_ is a plain static LogLevel: SetMinimumLevel writes it and IsEnabled (called first in Log) reads it without the lock or an atomic, so changing the level while another thread logs is a data race (undefined behaviour in C++, benign on common targets).
• Log calls the installed sink while holding SinkMutex(), a non-recursive std::mutex; a sink that logs through Logger, or calls SetSink or ResetSink, locks it again from the same thread, which is undefined behaviour and in practice a deadlock.
• An exception thrown by a sink propagates to the caller of Log (the lock is released).

Concurrent Log calls themselves are safe: formatting happens before the lock and the write happens under it.

Source locations

• modules/core/src/Logger.cpp — Logger::Log, IsEnabled, SetMinimumLevel, SetSink, ResetSink, SinkMutex
• modules/core/include/CNA/Logger.hpp — the public API, with no thread or re-entrancy statement
• modules/core/tests/CNA/LoggerTests.cpp — eleven LoggerTest cases; none concurrent or re-entrant

Evidence

Checked by reading at 009d40f5; nothing was built or executed for this entry. Both limits follow from the storage and locking in Logger.cpp. No thread-sanitizer run was made.

Focused reproduction

Illustrative; not compiled or run for this entry.

CNA::Logger::SetSink([](CNA::LogLevel, CNA::LogCategory, std::string_view line) {
    if (line.size() > 200)
        CNA::Logger::Warn("long log line");   // re-enters Log while SinkMutex() is held: deadlock
});
CNA::Logger::Info(std::string(300, 'x'));

Current tests

The eleven cases in LoggerTests.cpp cover the level gate, every level and category reaching the sink, the conditional overloads, the line format, ResetSink, stderr rather than stdout, and the level round trip. None runs two threads or installs a re-entrant sink.

Regression test

If the contract is widened: make minimumLevel_ atomic and add a thread-sanitizer test that logs from several threads while another changes the level; either call the sink outside the lock (copying it first) or reject re-entry, with a test for a sink that logs. If it is kept narrow, document both rules in Logger.hpp.

Blast radius

Programs that change the level while other threads (renderer, audio or loader threads) log, and sinks that forward into code that itself logs through CNA::Logger. Single-threaded configuration at start-up and plain concurrent logging are unaffected.

Workaround

Set the minimum level before starting other threads, and write sinks that never call CNA::Logger, SetSink or ResetSink (queue the line and emit it after the sink returns).