TSan: data race on the unsynchronized static FILE* lazy init in Plat_IsInDebugSession (Linux)

src/tier0/dbg.cpp の Plat_IsInDebugSession から開始し、steamnetworkingsockets_lowlevel_misc.cpp にある LockDebugInfo::AboutToUnlock からの呼び出しを追跡します。報告されたワークロードを ThreadSanitizer 下で再現し、その後、複数のスレッドが長時間保持されたロックを解放する場合に、初期化と /proc のステータス読み取りが安全であることを確認します。完了の条件は、競合が存在せず、デバッガーセッションのチェックが引き続き正しく機能することです。

Summary

On Linux, Plat_IsInDebugSession() caches its /proc/<pid>/status handle in a function-local static FILE * initialized by a plain test-and-set, with no synchronization:

src/tier0/dbg.cpp (master, L94-101):

#elif IsLinux()
	static FILE *fp;
	if ( !fp )                                     // <- unsynchronized read
	{
		char rgchProcStatusFile[256]; rgchProcStatusFile[0] = '\0';
		snprintf( rgchProcStatusFile, sizeof(rgchProcStatusFile), "/proc/%d/status", getpid() );
		fp = fopen( rgchProcStatusFile, "r" );     // <- unsynchronized write
	}

LockDebugInfo::AboutToUnlock() calls this from any thread that releases a lock it held past the long-lock warning threshold:

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_lowlevel_misc.cpp:

if ( usecElapsed >= t.m_usecLongLockWarningThreshold && !Plat_IsInDebugSession() )

So two threads — an application thread inside a public API call, and GNS's own SteamNetworkingThreadProc — can execute the lazy-init concurrently. ThreadSanitizer reports the 8-byte race on fp.

Worth noting this is not the thread-safe form. A function-local static with an initializer is guaranteed thread-safe since C++11; an assignment inside an if body is not, and gets no such guarantee.

Two distinct problems live in those lines:

1. The pointer. Both threads can see null, both fopen, and one handle leaks. This is what TSan flags.
2. The stream. Even once initialized, rewind(fp) followed by fgets(fp, ...) (L109-110) is not atomic as a pair. glibc locks each call individually, so the FILE is not corrupted, but two threads interleaving them read from each other's file position — a wrong answer to "am I under a debugger". Minor in consequence, but wrong.

This is unrelated to #419 / #424 (the PlayStation code path missing a return) and to #279 (efficiency of the same block), both of which leave the lazy init as it is.

Report

Reproduced under Clang 21.1.8 -fsanitize=thread on Linux (AlmaLinux 10.2, x86-64), against v1.6.0 (2cb93a06350bb065db53abdb0d87cf297e0bfd34), built from source via FetchContent. Current master (5f06b0a3c50be82297ccd012e5d6c298d90bfba7) carries the same implementation, so this is not fixed by upgrading.

Paths abbreviated to <GNS>. The application is a small authoritative game server; its only frame in the report is the flat-API call that was in progress.

WARNING: ThreadSanitizer: data race

  Read of size 8 by thread T1 (mutexes: write M0):
    #0 Plat_IsInDebugSession                      <GNS>/src/tier0/dbg.cpp:95
    #1 LockDebugInfo::AboutToUnlock()             <GNS>/.../steamnetworkingsockets_lowlevel_misc.cpp:321
    #2 Lock<std::recursive_timed_mutex>::unlock() <GNS>/.../steamnetworkingsockets_lowlevel.h:549
    #3 ScopeLock<ConnectionLock>::~ScopeLock()    <GNS>/.../steamnetworkingsockets_lowlevel.h:594
    #4 ConnectionScopeLock::~ConnectionScopeLock()<GNS>/.../steamnetworkingsockets_connections.h:344
    #5 CSteamNetworkingSockets::ReceiveMessagesOnConnection(...)
                                                  <GNS>/.../csteamnetworkingsockets.cpp:1425
    #6 SteamAPI_ISteamNetworkingSockets_ReceiveMessagesOnConnection
                                                  <GNS>/.../steamnetworkingsockets_flat.cpp:78
    #7 <application>::poll(...)                   <- the only application frame

  Previous write of size 8 by thread T2 (mutexes: write M1):
    #0 Plat_IsInDebugSession                      <GNS>/src/tier0/dbg.cpp:99
    #1 LockDebugInfo::AboutToUnlock()             <GNS>/.../steamnetworkingsockets_lowlevel_misc.cpp:321
    #2 Lock<std::recursive_timed_mutex>::unlock() <GNS>/.../steamnetworkingsockets_lowlevel.h:549
    #3 SteamNetworkingGlobalLock::Unlock()        <GNS>/.../steamnetworkingsockets_lowlevel_misc.cpp:445
    #4 PollRawUDPSockets(int, bool)               <GNS>/.../steamnetworkingsockets_socketthread.cpp:2646
    #5 SteamNetworkingSockets_InternalPoll(int, bool)
                                                  <GNS>/.../steamnetworkingsockets_socketthread.cpp:3320
    #6 SteamNetworkingThreadProc()                <GNS>/.../steamnetworkingsockets_socketthread.cpp:3438

SUMMARY: ThreadSanitizer: data race <GNS>/src/tier0/dbg.cpp:95 in Plat_IsInDebugSession

(Line 95 is the if ( !fp ) read and 99 the fp = fopen(...) write in the pinned v1.6.0 build; on current master the same two statements sit at 96 and 100.)

Reproduction

Nothing exotic — it only needs two threads to release a long-held lock at about the same time, which is what makes it show up under sanitizers rather than in release builds.

1. Build GNS with -fsanitize=thread (application and GNS both instrumented).
2. Run any workload with concurrent API traffic and a connection lifecycle — a server calling ReceiveMessagesOnConnection / SendMessages on one thread while GNS's service thread polls.
3. Wait for two threads to cross the long-lock warning threshold together.

Observed intermittently — 1 run in 6 of a four-client integration test. The frequency is a property of how often the branch is taken, not of the defect: under TSan, locks routinely exceed m_usecLongLockWarningThreshold, which is what opens this normally-rare path at all.

Suggested fix directions

Any of these would resolve it; I have no stake in which:

• A function-local static with an initializer, which C++11 makes thread-safe:

  static FILE *const fp = []() {
      char path[256];
      snprintf( path, sizeof(path), "/proc/%d/status", getpid() );
      return fopen( path, "r" );
  }();
  

• std::call_once with a std::once_flag, if the lambda form is not to taste.
• An atomic pointer (std::atomic<FILE *>, relaxed) if the double-fopen is considered acceptable and only the race needs removing.
• Or any equivalent synchronized initialization.

If the stream sharing is also a concern, making the whole body thread_local, or guarding rewind/fgets together with a small mutex, would address point 2 as well.

Whatever the choice, it would help downstream users if the fix (or a decision that the race is intended and benign) were visible, since every project running GNS under ThreadSanitizer currently has to rediscover and characterize this independently, and decide on its own whether it is looking at a library-internal issue or a bug in its own code.

Environment

GNS	v1.6.0, 2cb93a06350bb065db53abdb0d87cf297e0bfd34
Master checked	5f06b0a3c50be82297ccd012e5d6c298d90bfba7 — affected
Compiler	Clang 21.1.8, -fsanitize=thread
OS	AlmaLinux 10.2, x86-64, WSL2 kernel 6.18
Build	FetchContent, static OSS direct-IP client library, no ICE

Related but distinct: #419 / #424 (PlayStation return path), #279 (efficiency of the same block). Separately filed for this project: #429 (connection-state read outside the connection lock).