Celtoys / Remotery

Single C file, Realtime CPU/GPU Profiler with Remote Web Viewer
Apache License 2.0
3.03k stars 255 forks source link

Crash on shutdown if rmt_DestroyGlobalInstance is called right after initialization without delay. #252

Open ruifig opened 7 months ago

ruifig commented 7 months ago

I'm adding Remotery in an SDK I'm working on, and I found out that Remotery crashes if I don't have a delay after initialization.

This is the callstack:

>   LudeoSDK-Win64-Debug.dll!AtomicAddS32(volatile int * value, int add) Line 728   C
    LudeoSDK-Win64-Debug.dll!rmtGetThreadNameFallback(char * out_thread_name, unsigned int thread_name_size) Line 6256  C
    LudeoSDK-Win64-Debug.dll!rmtGetThreadName(unsigned long thread_id, void * thread_handle, char * out_thread_name, unsigned int thread_name_size) Line 2163   C
    LudeoSDK-Win64-Debug.dll!ThreadProfiler_Constructor(rmtMessageQueue * mq_to_rmt, ThreadProfiler * thread_profiler, unsigned long thread_id) Line 5224   C
    LudeoSDK-Win64-Debug.dll!ThreadProfilers_GetThreadProfiler(ThreadProfilers * thread_profilers, unsigned long thread_id, ThreadProfiler * * out_thread_profiler) Line 5520   C
    LudeoSDK-Win64-Debug.dll!GatherThreads(ThreadProfilers * thread_profilers) Line 5608    C
    LudeoSDK-Win64-Debug.dll!InitThreadSampling(ThreadProfilers * thread_profilers) Line 5874   C
    LudeoSDK-Win64-Debug.dll!SampleThreadsLoop(Thread_t * rmt_thread) Line 5907 C
    LudeoSDK-Win64-Debug.dll!ThreadProcWindows(void * lpParameter) Line 2228    C

It seems to me that if rmt_DestroyGlobalInstance is called fast enough, it catches some threads still initializing, which end up trying to use g_Remotery after it is set to NULL.

Not sure if there are any other implications, but maybe the fix is just changing some things in Remotery_Destructor ?

static void Remotery_Destructor(Remotery* rmt)
{
    assert(rmt != NULL);

    // Join the remotery thread before clearing the global object as the thread is profiling itself
    rmtDelete(rmtThread, rmt->thread);

    if (g_RemoteryCreated)
    {
        g_Remotery = NULL;
        g_RemoteryCreated = RMT_FALSE;
    }

    rmtDelete(ObjectAllocator, rmt->propertyAllocator);

    rmtDelete(ThreadProfilers, rmt->threadProfilers); <<< Crashing while waiting for this, because g_Remotery is NULL.

Moving the ThreadProfilers destruction to right after the rmtThread destruction seems to fix the issue. E.g:

static void Remotery_Destructor(Remotery* rmt)
{
    assert(rmt != NULL);

    // Join the remotery thread before clearing the global object as the thread is profiling itself
    rmtDelete(rmtThread, rmt->thread);
    rmtDelete(ThreadProfilers, rmt->threadProfilers);  <<<< Causes these ones to join before we reset g_RemoteryCreated
dwilliamson commented 7 months ago

I will look at this in more detail tomorrow, but if you have a head start, I seem to recall a previous issue that had a problem similar to this.

dwilliamson commented 7 months ago

Issues #214 and #208