[PATCH] D102684: [LLD] Allow disabling the early exit codepath as a build configuration

Sat May 22 13:23:58 PDT 2021

mstorsjo added a comment.

In D102684#2774826 <https://reviews.llvm.org/D102684#2774826>, @amccarth wrote:

> I could see how that might make even ExitProcess a risky proposition if there are any waits on synchronization objects happening during DLL_THREAD_DETACH.  Is this a well understood scenario?  If the DLL_THREAD_DETACH destructor problems are hard to fix, would it be feasible to have the thread pool thread signal the main thread to have _it_ call ExitProcess?

Having the threads signal the main thread and make the main thread call ExitProcess is pretty much the same as disabling the early exit mechanism, i.e. what this patch does (behind a configure option).

(The early exit flag does two things: First, exiting from any thread when the error limit is exceeded, vs waiting until the main thread checks if it should exit. And secondly, when exiting, both from the main thread and from worker threads, skip destructors or not. If we exit from non-main threads we really must not run destructors. If we exit from the main thread we could run all of them, but bypassing destructors in the same way shaves off some fraction of runtime, and makes the destructor bypass mechanism more tested.)

While exiting from a non-main thread is tricky, things also do hang when exiting from the main thread, when bypassing constructors. The fatal issue there is that we have a number of threads still running, that can keep various locks. When exiting, all other threads are stopped where they are, and we run the destructors for DLLs.

The hangs on exit look like this:

  * frame #0: 0x00007ffe068ae087 ntdll.dll`RtlAllocateHeap + 4215
    frame #1: 0x00007ffe068ad997 ntdll.dll`RtlAllocateHeap + 2439
    frame #2: 0x00007ffe029baeee ucrtbase.dll`_calloc_base + 78
    frame #3: 0x00007ffe029f94bc ucrtbase.dll`__intrinsic_setjmpex + 6012
    frame #4: 0x00007ffe029d142d ucrtbase.dll`_configthreadlocale + 13
    frame #5: 0x00007ffde79be2b7 libc++.dll`std::__1::ios_base::Init::~Init() + 167
    frame #6: 0x00007ffde79f8d5a libc++.dll`std::__1::strstreambuf::operator=(std::__1::strstreambuf&&) + 1418
    frame #7: 0x00007ffde79c7751 libc++.dll`std::__1::codecvt<wchar_t, char, _Mbstatet>::do_unshift(_Mbstatet&, char*, char*, char*&) const + 81
    frame #8: 0x00007ffde79beda9 libc++.dll`std::__1::ios_base::Init::~Init() + 2969
    frame #9: 0x00007ffde79b0016 libc++.dll`std::__1::basic_ostream<wchar_t, std::__1::char_traits<wchar_t> >::flush() + 86
    frame #10: 0x00007ffde79be1dc libc++.dll`std::__1::ios_base::Init::Init() + 124
    frame #11: 0x00007ffe029c3e26 ucrtbase.dll`_execute_onexit_table + 342
    frame #12: 0x00007ffe029c3d4b ucrtbase.dll`_execute_onexit_table + 123
    frame #13: 0x00007ffe029c3d04 ucrtbase.dll`_execute_onexit_table + 52
    frame #14: 0x00007ffde799109e libc++.dll`_CRT_INIT(hDllHandle=<unavailable>, dwReason=<unavailable>, lpreserved=<unavailable>) at crtdll.c:130:11
    frame #15: 0x00007ffde7991311 libc++.dll`__DllMainCRTStartup(hDllHandle=0x00007ffde7990000, dwReason=0, lpreserved=0x0000000000000001) at crtdll.c:195:6
    frame #16: 0x00007ffe068e8f07 ntdll.dll`RtlAnsiStringToUnicodeString + 663
    frame #17: 0x00007ffe068e35bc ntdll.dll`LdrShutdownProcess + 300
    frame #18: 0x00007ffe06906dcd ntdll.dll`RtlExitUserProcess + 173
    frame #19: 0x00007ffe042fd62a kernel32.dll`ExitProcess + 10
    frame #20: 0x00007ffe029d00a4 ucrtbase.dll`exit + 468
    frame #21: 0x00007ffe029cff4f ucrtbase.dll`exit + 127
    frame #22: 0x00000001400d2679 ld.lld.exe`_Exit + 9
    frame #23: 0x00000001400d265f ld.lld.exe`llvm::sys::Process::Exit(int, bool) + 31
    frame #24: 0x000000014003beac ld.lld.exe`lld::exitLld(int) + 460
    frame #25: 0x000000014004ad55 ld.lld.exe`lld::coff::link(llvm::ArrayRef<char const*>, bool, llvm::raw_ostream&, llvm::raw_ostream&) + 565
    frame #26: 0x00000001400ad85e ld.lld.exe`lld::mingw::link(llvm::ArrayRef<char const*>, bool, llvm::raw_ostream&, llvm::raw_ostream&) + 20046
    frame #27: 0x0000000140001f08 ld.lld.exe`lldMain(int, char const**, llvm::raw_ostream&, llvm::raw_ostream&, bool) + 1608

(The top two frames, viewed from WinDbg instead of lldb, are `ntdll!RtlpLowFragHeapAllocFromContext` and `ntdll!RtlpAllocateHeapInternal`.)

So while libc++ ideally wouldn't be calling `_configthreadlocale` when cleaning up, we're also pretty much in pretty bad place in any case if RtlAllocateHeap hangs.

> I'd go so far as to say that if you experience a deadlock or other problem with ExitProcess, the code likely has a bug or at least a questionable design choice.

Well the questionable design choice in this case is LLD that wants to exit while there's lots of threads still "running"/not joined. This is designed to be done by not running any destructors at all, but when we can have DLL boundaries at multiple places (around libc++, or around libLLVM) we end up running some (but not all) destructors.

I'll go ahead and push D102944 <https://reviews.llvm.org/D102944> which should make sure that no destructors whatsoever are called - hopefully that will put an end to these issues. If not we might need to just disable the early exit mechanism altogether on windows and avoid the whole questionable design that way.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102684/new/

https://reviews.llvm.org/D102684