[libcxx-commits] [PATCH] D155278: [libc++][mingw] TLS cleanup on windows

Tue Jul 18 05:52:45 PDT 2023

mstorsjo added a comment.

In D155278#4500509 <https://reviews.llvm.org/D155278#4500509>, @jrtc27 wrote:

> Isn't atexit for process termination? I'd be surprised if it really does run during FreeLibrary. The normal (/documented) approach is to handle DLL_PROCESS_DETACH inside DllMain, but many applications also seem to use a C++ destructor, which will compose better with static linking.

As @kalle-llvm already mentioned, atexit on Windows actually is hooked up to the DllMain handler for DLL_PROCESS_DETACH, and this is the underlying mechanism used for C++ destructors already. Handlers set up with atexit are executed either when the process exits or the current DLL gets unloaded, whichever happens first.

In D155278#4500535 <https://reviews.llvm.org/D155278#4500535>, @jrtc27 wrote:

> I'll also note libcxxabi/src/cxa_thread_atexit.cpp has some documented hairiness around `__libcpp_tls_create`, and that both it and `__thread_specific_ptr` are users of `__libcpp_tls_create` that presumably have similar issues to the one discussed here for `__cxa_get_globals`.

Thanks for the pointers. For this specific use (which is specific to mingw usecases; it's windows-only, and libcxxabi isn't used in MSVC-like environments, only in itanium environments) I think it might be ok to rely on details about the actual orderings on windows/mingw.

@kalle-llvm Am I right in assuming, that the main ordering problem to be concerned about here, is when this atexit-registered function is called when a process is exiting (as opposed to when the enclosing DLL gets unloaded), whether the `destroy_` function, calling `FlsFree`, gets called after the tls slot destructor itself already has fired (as I presume it does on shutdown)? I guess relevant cases to test is both when libc++ lives in its own DLL and when it is statically linked into the main executable (they have slightly different destructor execution orderings). If we can see that duplicate execution is safe here, or that we're sure that the atexit callback gets called before the tls slot destructors for the main thread, then this should probably be fine.

================
Comment at: libcxx/include/__threading_support:244
+_LIBCPP_THREAD_ABI_VISIBILITY
+void __libcpp_tls_destroy(__libcpp_tls_key __key);
+
----------------
Ok, so this is a new ABI entry in libcxx, but which only would be provided on certain platforms.

On one hand, this doesn't affect other platforms and their ABIs, and this is private API (used by libcxxabi) so it should probably be fine. But should we wrap it in a suitable ifdef to make it abundantly clear that it only is available in some cases?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155278