[libc++] Avoid allocations when the process is shutting down

[m417z]

This issue is motivated by a pretty nasty problem I had to debug on Windows. I'm not sure libc++ does something that's strictly wrong in this case, but I believe that it can improve and reduce the likelihood of such problems.

After several attempts, I reduced my problem to the following. The issue can be observed in Windows 10 by compiling the snippet with the llvm-mingw toolchain with the msvcrt runtime. I couldn't reproduce it on Windows 11, but a different variant might work.

// Compile: g++.exe test.cpp -target x86_64-w64-mingw32 -Wl,-subsystem,windows -o test.exe

#include <windows.h>

class TestDestruction {
   public:
    ~TestDestruction() {
        // Expected to be run and determine the process exit code.
        TerminateProcess(GetCurrentProcess(), 5678);
    }
};
TestDestruction x;

int WINAPI WinMain(HINSTANCE hInstance,
                   HINSTANCE hPrevInstance,
                   LPSTR lpCmdLine,
                   int nCmdShow) {
        SHELLEXECUTEINFOW sh{
            .cbSize = sizeof(sh),
            .lpVerb = L"open",
            .lpFile = L"shell:::{20D04FE0-3AEA-1069-A2D8-08002B30309D}",
            .nShow = SW_SHOWMINNOACTIVE,
        };
        ShellExecuteExW(&sh);

        ExitProcess(1234);
        return 0;
}

The code should return 5678, but sometimes it returns 1234, in which case ~TestDestruction never runs:

start /wait test.exe & echo %errorlevel%
1234

start /wait test.exe & echo %errorlevel%
5678

A more convenient test case which does several attempts:

Test case source code (click to expand)

#include <windows.h>

#include <locale.h>
#include <stdio.h>
#include <time.h>
#include <wchar.h>

class TestDestruction {
   public:
    ~TestDestruction() {
        // Expected to run and determine the process exit code.
        TerminateProcess(GetCurrentProcess(), 5678);
    }
};
TestDestruction x;

int WINAPI WinMain(HINSTANCE hInstance,
                   HINSTANCE hPrevInstance,
                   LPSTR lpCmdLine,
                   int nCmdShow) {
    if (lpCmdLine && lpCmdLine[0] == '1' && lpCmdLine[1] == '\0') {
        SHELLEXECUTEINFOW sh{
            .cbSize = sizeof(sh),
            .lpVerb = L"open",
            .lpFile = L"shell:::{20D04FE0-3AEA-1069-A2D8-08002B30309D}",
            .nShow = SW_SHOWMINNOACTIVE,
        };
        ShellExecuteExW(&sh);

        ExitProcess(1234);
        return 0;
    }

    WCHAR path[MAX_PATH];
    GetModuleFileNameW(nullptr, path, MAX_PATH);
    WCHAR cmdline[MAX_PATH + 16];
    wsprintfW(cmdline, L"\"%s\" 1", path);
    int i;
    for (i = 0; i < 20; i++) {
        STARTUPINFOW si = {sizeof(si)};
        PROCESS_INFORMATION pi;
        if (!CreateProcessW(path, cmdline, nullptr, nullptr, FALSE, 0, nullptr,
                            nullptr, &si, &pi)) {
            MessageBoxA(nullptr, "CreateProcessW", "Error", MB_ICONERROR);
            break;
        }

        WaitForSingleObject(pi.hProcess, INFINITE);

        DWORD exitCode;
        GetExitCodeProcess(pi.hProcess, &exitCode);
        if (exitCode != 5678) {
            char msg[256];
            wsprintfA(msg, "Unexpected exit code: %u", exitCode);
            MessageBoxA(nullptr, msg, "Error", MB_ICONERROR);
            break;
        }

        CloseHandle(pi.hProcess);
        CloseHandle(pi.hThread);

        Sleep(1000);
    }

    if (i == 20) {
        MessageBoxA(nullptr, "Failed to trigger bug in 20 attempts", "Hmm",
                    MB_ICONINFORMATION);
    } else {
        char msg[256];
        wsprintfA(msg, "Triggered bug in %u attempts", i + 1);
        MessageBoxA(nullptr, msg, "Success?", MB_ICONINFORMATION);
    }
}

TerminateProcess is only for a quick demonstration, it could also be something like database flushing which is problematic to be skipped.

Even worse, running this snippet may show an error message box to the user. It's always called in the buggy scenario, but usually the process terminates quickly enough and it's not visible. The stack trace in this case is the following:

0d user32!MessageBoxA+0x4e
0e msvcrt!_crtMessageBoxA+0x1fc
0f msvcrt!NMSG_WRITE+0x16a
10 msvcrt!amsg_exit+0x45
11 msvcrt!getptd+0x1b
12 msvcrt!setlocale+0x67
13 libc__!ZNSt3__18ios_base4InitD1Ev+0x1aa
14 libc__!ZTv0_n24_NSt3__19strstreamD0Ev+0x442
15 libc__!ZNKSt3__17codecvtIwciE10do_unshiftERiPcS3_RS3_+0x45
16 libc__!ZNSt3__18ios_base4InitD1Ev+0xe10
17 libc__!ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEE5flushEv+0x59
18 libc__!ZNSt3__18ios_base4InitC1Ev+0x88
19 libc__!_execute_onexit_table+0x4e [../misc/onexit_table.c @ 67] 
1a libc__!_CRT_INIT+0x8f [../crt/crtdll.c @ 131] 
1b libc__!__DllMainCRTStartup+0x157 [../crt/crtdll.c @ 196] 
1c ntdll!LdrpCallInitRoutine+0x61
1d ntdll!LdrShutdownProcess+0x22a
1e ntdll!RtlExitUserProcess+0xad
1f kernel32!ExitProcessImplementation+0xb
20 explorer!wWinMain+0xe6c
21 explorer!__scrt_common_main_seh+0x106
22 kernel32!BaseThreadInitThunk+0x14
23 ntdll!RtlUserThreadStart+0x21

After trying various things to try and explain the problem, here is what happens to the best of my understanding:

• ShellExecuteExW spawns extra threads, and these threads are still active in the background after it returns.
• ExitProcess is called when one of the background threads is using the CRT heap. This makes the heap unusable during process shutdown:

all the other threads in the process are forcible terminated
[...]
it can’t [allocate] because the heap lock was owned by another thread

https://devblogs.microsoft.com/oldnewthing/20120106-00/?p=8663

• During libc++ cleanup, setlocale is being called (likely via __libcpp_locale_guard).
• msvcrt's implementation of setlocale calls _getptd_noexit, which tries to get the per-thread data structure, but it might be already freed, so it tries to allocate it again.
• Allocation fails, in which case a message box is shown.

This is a pretty bad problem that can cause issues such as data corruption and bad user experience due to the flashing message box.

There are several factors that contribute to the problem, but the worst offender is probably msvcrt - not only does it treat memory allocation failure as fatal instead of returning NULL, it also shows a message box to the user.

Still, on libc++'s side, are the setlocale calls really necessary during process shutdown? If not, it can check whether the process is shutting down (via RtlDllShutdownInProgress or DllMain's lpvReserved value during DLL_PROCESS_DETACH), and do only the minimal necessary cleanup in this case.

And if the calls are necessary, is the code designed to handle failures gracefully? If setlocale returned NULL as it should in this case, I see that the code throws bad_alloc, I assume that it will cause the process shutdown to be aborted without proceeding with the other destructors.