[libcxx-commits] [libcxx] [libc++] Improve performance of std::atomic_flag on Windows (PR #163524)

[Roger Sanders via libcxx-commits]

================
@@ -101,6 +105,46 @@ static void __libcpp_platform_wake_by_address(__cxx_atomic_contention_t const vo
   _umtx_op(const_cast<__cxx_atomic_contention_t*>(__ptr), UMTX_OP_WAKE, __notify_one ? 1 : INT_MAX, nullptr, nullptr);
 }
 
+#elif defined(_WIN32)
+
+static void
+__libcpp_platform_wait_on_address(__cxx_atomic_contention_t const volatile* __ptr, __cxx_contention_t __val) {
+  // WaitOnAddress was added in Windows 8 (build 9200)
+  static auto wait_on_address = reinterpret_cast<BOOL(WINAPI*)(volatile void*, PVOID, SIZE_T, DWORD)>(
+      GetProcAddress(GetModuleHandleW(L"api-ms-win-core-synch-l1-2-0.dll"), "WaitOnAddress"));
+  if (wait_on_address != nullptr) {
----------------
RogerSanders wrote:

> Good point, I forgot that LoadLibrary is scary in DllMain. Fair chance user code calls this in a global ctor, and even if not, `static auto module_handle`'s dtor will run in DllMain; FreeLibrary [is explicitly documented](https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-freelibrary) as unsafe.

I don't consider the LoadLibrary call a real problem. I can't imagine anyone attempting to call notify or wait on atomic from a global static constructor, and if they did, it's already undefined for a bunch of other reasons. What threads are you synchronising with, and how could you safely create them and interact with them from a global static constructor? From both an OS and c++ language level, you're already in undefined territory.

The FreeLibrary case though is a significantly bigger issue. 

I hate to say it, but we could just leak the handle? So KernelBase.dll doesn't get automatically unloaded if you dynamically load, then unload, libc++. Well KernelBase.dll is loaded by default anyway, so removing our reference does nothing. Who is dynamically binding to libc++, who absolutely needs KernelBase to have its reference count drop when libc++ is unloaded?

It blows though. Totally understand if leaking a module handle wouldn't pass, even such a core one.

> GetModuleHandleEx (and GetProcAddress) are not documented unsafe, but absense of evidence doesn't prove anything.

Those are definitely safe in DllMain, as all but a few listed functions from kernel32.dll are explicitly mentioned as safe to call:
https://learn.microsoft.com/en-us/windows/win32/Dlls/dynamic-link-library-best-practices

https://github.com/llvm/llvm-project/pull/163524