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

[Andrew Ng via Phabricator via llvm-commits]

andrewng added a comment.

I looked into this in some detail back in the day and am mainly responsible for how it currently functions. I primarily focused on static linking with the MSVC static runtime, as that's what we use downstream and were seeing issues with, but also included MinGW, GNU and libc++. To be honest, I can't remember if I tried LLVM_LINK_LLVM_DYLIB, there are just too many possible configurations! The various compilers/runtimes have changed since I last looked at this code, so it's not surprising that there may be issues depending on your configuration.

>From past experience, I think it has always been "dangerous" on Windows to exit without waiting/terminating any running threads. If this can't be done, then a "hard" exit, e.g. via ExitProcess(), is usually the only reliable way to finish and I this is what we currently still use downstream for the LLD early exit. IIRC, although this may have now changed, the LLD lit testsuite disables early exit for running the tests (at least for ELF) and it was actually this scenario with the MSVC debug static runtime and MS ConcRT (used at that time) that was intermittently crashing on exit during lit testing.

The other thing that complicates the situation is that LLD can exit from a thread from the thread pool in certain error cases, i.e. doesn't always exit from the main thread. This scenario is going to be rare in actual practical use, but AFAIK is still a path exercised by the LLD testing.

So in terms of how to "fix" this, I would say that disabling early exit for Windows is definitely an option, assuming the impact for high core counts isn't too significant. Or perhaps using ExitProcess() for early exit which has worked for us downstream, at least for static linking with the MSVC static runtime. The trickiest part is really going to be checking any "fix" works for the configurations that people care about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102684