Skip to content

Commit 886a0ab

Browse files
authored
Merge pull request wolfpld#1215 from slomp/slomp/worker-thread-race
Fixed race condition around `s_sysTraceThread`
2 parents 52bfac4 + 7fa30d0 commit 886a0ab

File tree

1 file changed

+19
-11
lines changed

1 file changed

+19
-11
lines changed

public/client/TracyProfiler.cpp

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,7 @@ static Thread* s_symbolThread;
933933
std::atomic<bool> s_symbolThreadGone { false };
934934
#endif
935935
#ifdef TRACY_HAS_SYSTEM_TRACING
936-
static Thread* s_sysTraceThread = nullptr;
936+
static std::atomic<Thread*> s_sysTraceThread = nullptr;
937937
#endif
938938

939939
#if defined __linux__ && !defined TRACY_NO_CRASH_HANDLER
@@ -1177,27 +1177,29 @@ static void StartSystemTracing( int64_t& samplingPeriod )
11771177
// use TRACY_NO_SYS_TRACE=1 to force disabling sys tracing (even if available in the underlying system)
11781178
// as it can have significant impact on the size of the traces
11791179
const char* noSysTrace = GetEnvVar( "TRACY_NO_SYS_TRACE" );
1180-
const bool disableSystrace = (noSysTrace && noSysTrace[0] == '1');
1180+
const bool disableSystrace = ( noSysTrace && noSysTrace[0] == '1' );
11811181
if( disableSystrace )
11821182
{
1183-
TracyDebug("TRACY: Sys Trace was disabled by 'TRACY_NO_SYS_TRACE=1'\n");
1183+
TracyDebug( "TRACY: Sys Trace was disabled by 'TRACY_NO_SYS_TRACE=1'\n" );
11841184
}
11851185
else if( SysTraceStart( samplingPeriod ) )
11861186
{
1187-
s_sysTraceThread = (Thread*)tracy_malloc( sizeof( Thread ) );
1188-
new(s_sysTraceThread) Thread( SysTraceWorker, nullptr );
1187+
Thread* sysTraceThread = (Thread*)tracy_malloc( sizeof( Thread ) );
1188+
new( sysTraceThread ) Thread( SysTraceWorker, nullptr );
1189+
Thread* prev = s_sysTraceThread.exchange( sysTraceThread );
1190+
assert( prev == nullptr );
11891191
std::this_thread::sleep_for( std::chrono::milliseconds( 1 ) );
11901192
}
11911193
}
11921194

11931195
static void StopSystemTracing()
11941196
{
1195-
if( s_sysTraceThread )
1197+
Thread* sysTraceThread = s_sysTraceThread.exchange( nullptr );
1198+
if( sysTraceThread )
11961199
{
11971200
SysTraceStop();
1198-
s_sysTraceThread->~Thread();
1199-
tracy_free( s_sysTraceThread );
1200-
s_sysTraceThread = nullptr;
1201+
sysTraceThread->~Thread();
1202+
tracy_free( sysTraceThread );
12011203
}
12021204
}
12031205
#endif
@@ -2146,9 +2148,15 @@ void Profiler::Worker()
21462148
while( s_symbolThreadGone.load() == false ) { YieldThread(); }
21472149
#endif
21482150

2149-
// Client is exiting.
21502151
#ifdef TRACY_HAS_SYSTEM_TRACING
2151-
// Stop filling queues with new data.
2152+
// On a typical shutdown scenario, the (global) Profiler object is destroyed by
2153+
// the C++ runtime when the client program returns from "main", and ~Profiler()
2154+
// takes care of calling StopSystemTracing(). However, a client may decide to
2155+
// manually RequestShutdown(), in which case ~Profile() may not execute before
2156+
// this Worker() thread goes through its teardown stages and reaches this point.
2157+
// To ensure that system tracing does not keep pushing data to the worker queue
2158+
// indefinitely (thus preventing this worker from terminating), we have to call
2159+
// StopSystemTracing() here as well to be safe:
21522160
StopSystemTracing();
21532161
#endif
21542162

0 commit comments

Comments
 (0)