Working around a problem with msvcrt and `setlocale`

[m417z]

I wrote here in detail about a nasty problem I encountered with msvcrt: llvm/llvm-project#110954

The short version is that if allocation fails while calling setlocale, which may happen in Windows during process shutdown regardless of memory availability, msvcrt shows a message box and terminates the process, both of which are pretty bad.

I'm not sure how the libc++ maintainers will react to the issue I created, I won't be surprised if they'll decide it's out of scope and an msvcrt problem. Since this toolchain is designed to work with msvcrt, perhaps a workaround can be added in the toolchain or its version of libc++. For example, setlocale can be wrapped by something like:

char* setlocale_wrapper(int category, const char* locale) {
  _ptiddata ptd = _getptd_noexit();
  return ptd ? setlocale(category, locale) : NULL;
}

Unfortunately _getptd_noexit isn't exported by msvcrt, so the actual implementation must be different.
_get_errno always returns ENOMEM in this case, and _set_errno always fails with ENOMEM, so an actual implementation can be:

char* setlocale_wrapper(int category, const char* locale) {
  errno_t err;
  if (_get_errno(&err) == 0 && err == ENOMEM && _set_errno(err) == ENOMEM) {
    return NULL;
  }
  return setlocale(category, locale);
}

It's not the most elegant solution but it solves a real problem.

What do you think? Any other ideas?

Thanks, and thank you for this project.

[mstorsjo]

I wrote here in detail about a nasty problem I encountered with msvcrt: llvm/llvm-project#110954

Thanks for the detailed bug report and analysis!

I'm not sure how the libc++ maintainers will react to the issue I created, I won't be surprised if they'll decide it's out of scope and an msvcrt problem. Since this toolchain is designed to work with msvcrt, perhaps a workaround can be added in the toolchain or its version of libc++.

To set the expectations right here, it's not so much that this toolchain is designed to work with msvcrt; I try to make sure that configuration works, but it's definitely a second priority for the toolchain.

And within llvm-mingw, I keep a fairly strict upstream-only/no-patches policy; I don't patch the sources from llvm-project/mingw-w64, but make sure to do fixes upstream - so they can benefit all distributions. (But when needed, I can do tweaks in how the code is built.) Specific workarounds for msvcrt.dll are, unfortunately, in a bit of a tricky spot wrt that though.

Is this issue only present with msvcrt by design, or is it possible that UCRT could have similar issues as well, just not as easy to trigger?

Does setlocale() really work at all in msvcrt.dll? If not, it could be an option to build libc++ for msvcrt in a way that just tries not to use setlocale() at all.

Avoiding calling setlocale() during process shutdown sounds like something that could be reasonable in itself, but I'm not sure how keen libc++ would be on accepting such a workaround upstream. How does RtlDllShutdownInProgress as mentioned in the upstream bug report compare to the hacky ways around it that you presented here?

It could in theory be possible to add wrappers in mingw-w64, to make setlocale() safer in these circumstances, but that also feels like a quite long workaround, when it's primarily an issue in libc++, where it shouldn't be calling that to begin with.

I browsed https://devblogs.microsoft.com/oldnewthing/20120105-00/?p=8683 (which was referenced from https://devblogs.microsoft.com/oldnewthing/20120106-00/?p=8663 which you linked); when we're exiting the process we shouldn't be doing anything at all. But doesn't that also mean that we shouldn't be running any C++ destructors at all? Or put even further, that we shouldn't be running any atexit()-handlers or similar? Or do they run first, before we enter the "process is terminating" state?

Does it make any difference here, if libc++ is linked statically or dynamically? It somewhat changes the execution order of destructors, IIRC.

[alvinhochun]

I think the thing is that you should prefer std::exit over ExitProcess to allow global destructors to run before the actual ExitProcess call, which Raymond Chen explained in "When are global objects constructed and destructed by Visual C++?, redux".

[m417z]

Is this issue only present with msvcrt by design, or is it possible that UCRT could have similar issues as well, just not as easy to trigger?

The message box behavior is part of msvcrt. I just tried UCRT now, the behavior looks consistent unlike msvcrt, but slightly different - if I use return to exit the program, the destructor always runs, as expected. If I use ExitProcess to exit the program, the destructor doesn't run unlike with msvcrt. That's also how MSVC behaves, and MinGW GCC too, so that seems OK.

That's also what Raymond is saying in a follow-up blog post:

I believe that more recent versions of the C runtime library [...] say, “You know what? If you exit the process by calling ExitProcess, then I’m simply not going to destruct anything. [...]"

The above is for exes. With a dll it's more tricky to reproduce, as the destruction order depends on the load order of the DLLs, so to test whether a destructor runs or is aborted, I need to make sure the crt dll is loaded after the test dll. I tested it with an additional MSVC dll and it seems similarly problematic with msvcrt.

UCRT has a similar problem if allocations are forced to fail - it aborts the process shutdown. At least it doesn't trigger a message box. The stack is without symbols, but configthreadlocale and abort are exported and correct:

ucrtbase.abort+4E
ucrtbase.<no-symbol>
ucrtbase._configthreadlocale+D
libc++.std::__1::ios_base::Init::~Init()+1A7
libc++.__divsc3+792
libc++.std::__1::codecvt<wchar_t, char, _Mbstatet>::do_unshift(_Mbstatet&, char*, char*, char*&) const+45
libc++.std::__1::ios_base::Init::~Init()+E80
libc++.std::__1::basic_ostream<wchar_t, std::__1::char_traits<wchar_t> >::flush()+59
libc++.std::__1::ios_base::Init::Init()+88
ucrtbase._execute_onexit_table+156
ucrtbase._execute_onexit_table+7B
ucrtbase._execute_onexit_table+34
libc++.00007FFA2A18109F
libc++.00007FFA2A181327
ntdll.LdrpCallInitRoutine+61
ntdll.LdrShutdownProcess+22A
ntdll.RtlExitUserProcess+AD
JMP.&ExitProcess

To summarize, in case of inability to allocate on process shutdown:

• 🔴 msvcrt, exe, return/ExitProcess: error message box, cleanup is aborted.
• 🔴 msvcrt, dll: error message box, cleanup is aborted (but cleanup of dlls using the lib is running before the error).
• 🟢 UCRT, exe, return: cleanup runs before process shutdown.
• 🟢 UCRT, exe, ExitProcess: no cleanup.
• 🟡 UCRT, dll, return/ExitProcess: cleanup is aborted, no message box.

Does setlocale() really work at all in msvcrt.dll?

The function exists and has an implementation (e.g. here for some version) so I assume it works at least to some extent.

Avoiding calling setlocale() during process shutdown sounds like something that could be reasonable in itself

Yes, depending on why it's being used. If, for example, it's necessary to handle some strings or file names to flush data to disk/stdout, it might be justified.

How does RtlDllShutdownInProgress as mentioned in the upstream bug report compare to the hacky ways around it that you presented here?

I assume you're thinking:

char* setlocale_wrapper(int category, const char* locale) {
  if (RtlDllShutdownInProgress()) {
    return NULL;
  }
  return setlocale(category, locale);
}

That means setlocale will always return NULL on process shutdown, not only in the rare cases when allocations aren't possible. It's more deterministic, but I'm afraid it's too destructive. As I mentioned upstream, from the code it looks like libc++ throws bad_alloc in this case, which I assume causes the process to abort the cleanup and terminate.

when we're exiting the process we shouldn't be doing anything at all. But doesn't that also mean that we shouldn't be running any C++ destructors at all? Or put even further, that we shouldn't be running any atexit()-handlers or similar? Or do they run first, before we enter the "process is terminating" state?

According to C++ it seems that destructors for global and static variables must be called. Raymond has a blog post about it as well, and a follow-up I already linked.

"Or do they run first, before we enter the "process is terminating" state?" - the answer seems to be yes for exes, and no for dlls which are still expected run destructors in the "process is terminating" state.

Does it make any difference here, if libc++ is linked statically or dynamically? It somewhat changes the execution order of destructors, IIRC.

My experience so far is only with a dynamically-linked libc++. The behavior is likely to be different with a statically-linked libc++, as in my tests the destruction flow starts with libc++.dll's DllMain.

[m417z]

@alvinhochun

I think the thing is that you should prefer std::exit over ExitProcess to allow global destructors to run before the actual ExitProcess call, which Raymond Chen explained in "When are global objects constructed and destructed by Visual C++?, redux".

In the case of an exe, yes. My problem was with loading a dll into a process that I don't control. So it's bad if my dll starts disrupting another program's cleanup flow, and it's especially bad if it causes a message box to show up when the process shuts down.

[m417z]

I also posted a suggestion for UCRT to avoid freeing the per-thread data on process shutdown, which will make this issue much less likely.
https://developercommunity.visualstudio.com/t/UCRT:-Dont-free-Per-Thread-Data-PTD-i/10760651