IoThreadHolder edge case

[codecat]

Okay, so my situation might be a little bit specific, but perhaps can be solved without requiring me to use DISCORD_DISABLE_IO_THREAD. (This is actually the second project I've experienced this issue with, but only now actually figured out exactly what is happening.)

I build the discord-rpc library statically and link it into a dll, which I then dynamically load into a process.

This works great when cleanly shutting down with Discord_Shutdown(), but not on process termination. In this case, ~IoThreadHolder() gets called (because of the static instance of IoThreadHolder), which runs Stop() which causes a deadlock because it's waiting on a non-existing thread that has already been terminated:

All of the threads in the process, except the calling thread, terminate their execution without receiving a DLL_THREAD_DETACH notification.

The MSDN page also states:

If one of the terminated threads in the process holds a lock and the DLL detach code in one of the loaded DLLs attempts to acquire the same lock, then calling ExitProcess results in a deadlock.

The same thing also happens when trying to call Discord_Shutdown() in DllMain when lpvReserved is non-null, but that can be avoided by simply not calling that, as process termination here is essentially not clean.

I know this might be sort of an edge-case for me, but perhaps this can be fixed somehow?

[codecat]

I found a workaround for this that I'll be using for the time being:

    void Stop()
    {
#ifdef _MSC_VER
        // Windows-only hack: don't bother if thread is already killed
        DWORD dwExitCode = 0;
        ::GetExitCodeThread((HANDLE)ioThread.native_handle(), &dwExitCode);
        if (dwExitCode != STILL_ACTIVE) {
            return;
        }
#endif

        keepRunning.exchange(false);
        Notify();
        if (ioThread.joinable()) {
            ioThread.join();
        }
    }

[msciotti]

Thank you for letting us know about this and posting a temporary workaround for folks that may also be experiencing this.

@snowyote will take a look and see if there's a way that we can fix this a bit more elegantly.

[snowyote]

Honestly, it seems like we just shouldn't call Stop() from ~IoThreadHolder() - if we're at the point that static destructors have been called, pokily going to the trouble of shutting down threads seems pointless as the OS is obviously already doing it for us.

@codecat, does that seem like that would fix your problem without interfering with the Discord_Shutdown() use case?

[crmarsh]

iirc I added it because there is an abort in std::thread destructor if you didn't join the thread.

[codecat]

What @crmarsh says is correct as far I know, if you don't join a thread, the runtime starts to yell at you when the program finishes, which is why it's probably not a great idea to just remove the destructor.

Perhaps it's acceptable to have an extra optional parameter to Discord_Shutdown (or a different function altogether) to define whether the process is terminating or not? That way the IoThreadHolder can be marked as already shut down by the time the destructor gets called.

Only thing then is that you have to call the shutdown function yourself on process termination, but I guess that's acceptable in my case.

[janisozaur]

We (OpenRCT2) have also ran into this issue, which is basically static initialisation order fiasco.