Skip to content

Commit a7a115f

Browse files
committed
Add ATTR_NO_SANITIZE_THREAD and use it to silence a tsan warning
1 parent 380c380 commit a7a115f

3 files changed

Lines changed: 24 additions & 2 deletions

File tree

src/public/minbase/minbase_decls.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,17 @@
156156
#endif
157157
#endif
158158

159+
// Suppress ThreadSanitizer instrumentation for a function.
160+
// Use this for functions that intentionally access shared data without holding a lock,
161+
// where the access is known to be safe (e.g. a lockless hint-read with a locked slow path).
162+
// Prefer this over making the variable std::atomic<>, which would suppress TSan on ALL
163+
// accesses to the variable and could mask unintentional races elsewhere.
164+
#ifdef __SANITIZE_THREAD__
165+
#define ATTR_NO_SANITIZE_THREAD __attribute__(( no_sanitize_thread ))
166+
#else
167+
#define ATTR_NO_SANITIZE_THREAD
168+
#endif
169+
159170
// We have some template functions declared in header files that
160171
// only need static scope. Older MSVC version lets you use 'static' qualifiers
161172
// but GCC and new MSVC does not, where we need to use 'inline' to avoid multiple definitions.

src/steamnetworkingsockets/steamnetworkingsockets_thinker.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,18 @@ void IThinker::InternalEnsureMinThinkTime( SteamNetworkingMicroseconds usecTarge
7676
s_mutexThinkerTable.unlock();
7777
}
7878

79-
void IThinker::SetNextThinkTime( SteamNetworkingMicroseconds usecTargetThinkTime )
79+
ATTR_NO_SANITIZE_THREAD void IThinker::SetNextThinkTime( SteamNetworkingMicroseconds usecTargetThinkTime )
8080
{
81+
// Fast-path: check whether the schedule is already set to the requested time before acquiring
82+
// the thinker mutex. This read of m_usecNextThinkTime is intentionally lockless -- the service
83+
// thread may write it concurrently. The race is safe because:
84+
// 1. The read is a hint only. A stale or torn value just causes a spurious fall-through to
85+
// InternalSetNextThinkTime, which re-checks under the lock. There is no state corruption.
86+
// 2. On all supported 64-bit platforms the aligned 64-bit load is atomic at the hardware level,
87+
// so a torn read is not possible in practice.
88+
// We use ATTR_NO_SANITIZE_THREAD rather than making m_usecNextThinkTime std::atomic<> because
89+
// std::atomic would suppress TSan on every access to the variable, masking any future
90+
// unintentional races. The attribute suppresses only this one known-safe lockless access.
8191
if ( usecTargetThinkTime == m_usecNextThinkTime )
8292
return;
8393
s_mutexThinkerTable.lock();

src/steamnetworkingsockets/steamnetworkingsockets_thinker.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ class IThinker
3131

3232
/// Adjust schedule time to the earlier of the current schedule time,
3333
/// or the given time.
34-
inline void EnsureMinThinkTime( SteamNetworkingMicroseconds usecTargetThinkTime )
34+
ATTR_NO_SANITIZE_THREAD inline void EnsureMinThinkTime( SteamNetworkingMicroseconds usecTargetThinkTime )
3535
{
36+
// Lockless fast-path read -- see SetNextThinkTime for explanation of the intentional race.
3637
if ( usecTargetThinkTime < m_usecNextThinkTime )
3738
InternalEnsureMinThinkTime( usecTargetThinkTime );
3839
}

0 commit comments

Comments
 (0)