JonathanBrouwer
diff --git a/‎library/std/src/sys/pal/windows/c.rs‎
Lines changed: 5 additions & 1 deletion b/‎library/std/src/sys/pal/windows/c.rs‎
Lines changed: 5 additions & 1 deletion
diff --git a/‎library/std/src/sys/pal/windows/c/bindings.txt‎
Lines changed: 7 additions & 0 deletions b/‎library/std/src/sys/pal/windows/c/bindings.txt‎
Lines changed: 7 additions & 0 deletions
diff --git a/‎library/std/src/sys/pal/windows/c/windows_sys.rs‎
Lines changed: 8 additions & 0 deletions b/‎library/std/src/sys/pal/windows/c/windows_sys.rs‎
Lines changed: 8 additions & 0 deletions
diff --git a/‎library/std/src/sys/thread_local/guard/windows.rs‎
Lines changed: 200 additions & 85 deletions b/‎library/std/src/sys/thread_local/guard/windows.rs‎
Lines changed: 200 additions & 85 deletions
@@ -5,7 +5,7 @@
 #![unstable(issue = "none", feature = "windows_c")]
 #![allow(clippy::style)]
 
-use core::ffi::{CStr, c_uint, c_ulong, c_ushort, c_void};
+use core::ffi::{CStr, c_int, c_uint, c_ulong, c_ushort, c_void};
 use core::ptr;
 
 mod windows_sys;
@@ -241,3 +241,7 @@ cfg_select! {
 // Only available starting with Windows 8.
 #[cfg(not(target_vendor = "win7"))]
 windows_link::link!("ws2_32.dll" "system" fn GetHostNameW(name : PWSTR, namelen : i32) -> i32);
+
+unsafe extern "C" {
+    pub fn atexit(cb: unsafe extern "C" fn()) -> c_int;
+}
@@ -2130,6 +2130,11 @@ FindExSearchNameMatch
 FindFirstFileExW
 FindNextFileW
 FIONBIO
+FLS_OUT_OF_INDEXES
+FlsAlloc
+FlsFree
+FlsGetValue
+FlsSetValue
 FlushFileBuffers
 FORMAT_MESSAGE_ALLOCATE_BUFFER
 FORMAT_MESSAGE_ARGUMENT_ARRAY
@@ -2258,6 +2263,7 @@ IPV6_DROP_MEMBERSHIP
 IPV6_MREQ
 IPV6_MULTICAST_LOOP
 IPV6_V6ONLY
+IsThreadAFiber
 LINGER
 listen
 LocalFree
@@ -2318,6 +2324,7 @@ OPEN_ALWAYS
 OPEN_EXISTING
 OpenProcessToken
 OVERLAPPED
+PFLS_CALLBACK_FUNCTION
 PIPE_ACCEPT_REMOTE_CLIENTS
 PIPE_ACCESS_DUPLEX
 PIPE_ACCESS_INBOUND
 
@@ -27,6 +27,10 @@ windows_link::link!("kernel32.dll" "system" fn ExitProcess(uexitcode : u32) -> !
 windows_link::link!("kernel32.dll" "system" fn FindClose(hfindfile : HANDLE) -> BOOL);
 windows_link::link!("kernel32.dll" "system" fn FindFirstFileExW(lpfilename : PCWSTR, finfolevelid : FINDEX_INFO_LEVELS, lpfindfiledata : *mut core::ffi::c_void, fsearchop : FINDEX_SEARCH_OPS, lpsearchfilter : *const core::ffi::c_void, dwadditionalflags : FIND_FIRST_EX_FLAGS) -> HANDLE);
 windows_link::link!("kernel32.dll" "system" fn FindNextFileW(hfindfile : HANDLE, lpfindfiledata : *mut WIN32_FIND_DATAW) -> BOOL);
+windows_link::link!("kernel32.dll" "system" fn FlsAlloc(lpcallback : PFLS_CALLBACK_FUNCTION) -> u32);
+windows_link::link!("kernel32.dll" "system" fn FlsFree(dwflsindex : u32) -> BOOL);
+windows_link::link!("kernel32.dll" "system" fn FlsGetValue(dwflsindex : u32) -> *mut core::ffi::c_void);
+windows_link::link!("kernel32.dll" "system" fn FlsSetValue(dwflsindex : u32, lpflsdata : *const core::ffi::c_void) -> BOOL);
 windows_link::link!("kernel32.dll" "system" fn FlushFileBuffers(hfile : HANDLE) -> BOOL);
 windows_link::link!("kernel32.dll" "system" fn FormatMessageW(dwflags : FORMAT_MESSAGE_OPTIONS, lpsource : *const core::ffi::c_void, dwmessageid : u32, dwlanguageid : u32, lpbuffer : PWSTR, nsize : u32, arguments : *const *const i8) -> u32);
 windows_link::link!("kernel32.dll" "system" fn FreeEnvironmentStringsW(penv : PCWSTR) -> BOOL);
@@ -67,6 +71,7 @@ windows_link::link!("kernel32.dll" "system" fn GetWindowsDirectoryW(lpbuffer : P
 windows_link::link!("kernel32.dll" "system" fn InitOnceBeginInitialize(lpinitonce : *mut INIT_ONCE, dwflags : u32, fpending : *mut BOOL, lpcontext : *mut *mut core::ffi::c_void) -> BOOL);
 windows_link::link!("kernel32.dll" "system" fn InitOnceComplete(lpinitonce : *mut INIT_ONCE, dwflags : u32, lpcontext : *const core::ffi::c_void) -> BOOL);
 windows_link::link!("kernel32.dll" "system" fn InitializeProcThreadAttributeList(lpattributelist : LPPROC_THREAD_ATTRIBUTE_LIST, dwattributecount : u32, dwflags : u32, lpsize : *mut usize) -> BOOL);
+windows_link::link!("kernel32.dll" "system" fn IsThreadAFiber() -> BOOL);
 windows_link::link!("kernel32.dll" "system" fn LocalFree(hmem : HLOCAL) -> HLOCAL);
 windows_link::link!("kernel32.dll" "system" fn LockFileEx(hfile : HANDLE, dwflags : LOCK_FILE_FLAGS, dwreserved : u32, nnumberofbytestolocklow : u32, nnumberofbytestolockhigh : u32, lpoverlapped : *mut OVERLAPPED) -> BOOL);
 windows_link::link!("kernel32.dll" "system" fn MoveFileExW(lpexistingfilename : PCWSTR, lpnewfilename : PCWSTR, dwflags : MOVE_FILE_FLAGS) -> BOOL);
@@ -2667,6 +2672,7 @@ impl Default for FLOATING_SAVE_AREA {
         unsafe { core::mem::zeroed() }
     }
 }
+pub const FLS_OUT_OF_INDEXES: u32 = 4294967295u32;
 pub const FORMAT_MESSAGE_ALLOCATE_BUFFER: FORMAT_MESSAGE_OPTIONS = 256u32;
 pub const FORMAT_MESSAGE_ARGUMENT_ARRAY: FORMAT_MESSAGE_OPTIONS = 8192u32;
 pub const FORMAT_MESSAGE_FROM_HMODULE: FORMAT_MESSAGE_OPTIONS = 2048u32;
@@ -3037,6 +3043,8 @@ pub struct OVERLAPPED_0_0 {
 }
 pub type PCSTR = *const u8;
 pub type PCWSTR = *const u16;
+pub type PFLS_CALLBACK_FUNCTION =
+    Option<unsafe extern "system" fn(lpflsdata: *const core::ffi::c_void)>;
 pub type PIO_APC_ROUTINE = Option<
     unsafe extern "system" fn(
         apccontext: *mut core::ffi::c_void,
 
@@ -1,103 +1,218 @@
 //! Support for Windows TLS destructors.
 //!
-//! Unfortunately, Windows does not provide a nice API to provide a destructor
-//! for a TLS variable. Thus, the solution here ended up being a little more
-//! obscure, but fear not, the internet has informed me [1][2] that this solution
-//! is not unique (no way I could have thought of it as well!). The key idea is
-//! to insert some hook somewhere to run arbitrary code on thread termination.
-//! With this in place we'll be able to run anything we like, including all
-//! TLS destructors!
+//! Windows has an API to provide a destructor for a FLS (fiber local storage) variable,
+//! which behaves similarly to a TLS variable for our purpose [1].
 //!
-//! In order to realize this, all TLS destructors are tracked by *us*, not the
-//! Windows runtime. This means that we have a global list of destructors for
+//! All TLS destructors are tracked by *us*, not the Windows runtime.
+//! This means that we have a global list of destructors for
 //! each TLS key or variable that we know about.
 //!
-//! # What's up with CRT$XLB?
-//!
-//! For anything about TLS destructors to work on Windows, we have to be able
-//! to run *something* when a thread exits. To do so, we place a very special
-//! static in a very special location. If this is encoded in just the right
-//! way, the kernel's loader is apparently nice enough to run some function
-//! of ours whenever a thread exits! How nice of the kernel!
-//!
-//! Lots of detailed information can be found in source [1] above, but the
-//! gist of it is that this is leveraging a feature of Microsoft's PE format
-//! (executable format) which is not actually used by any compilers today.
-//! This apparently translates to any callbacks in the ".CRT$XLB" section
-//! being run on certain events.
-//!
-//! So after all that, we use the compiler's `#[link_section]` feature to place
-//! a callback pointer into the magic section so it ends up being called.
-//!
-//! # What's up with this callback?
-//!
-//! The callback specified receives a number of parameters from... someone!
-//! (the kernel? the runtime? I'm not quite sure!) There are a few events that
-//! this gets invoked for, but we're currently only interested on when a
-//! thread or a process "detaches" (exits). The process part happens for the
-//! last thread and the thread part happens for any normal thread.
-//!
-//! # The article mentions weird stuff about "/INCLUDE"?
-//!
-//! It sure does! Specifically we're talking about this quote:
-//!
-//! ```quote
-//! The Microsoft run-time library facilitates this process by defining a
-//! memory image of the TLS Directory and giving it the special name
-//! “__tls_used” (Intel x86 platforms) or “_tls_used” (other platforms). The
-//! linker looks for this memory image and uses the data there to create the
-//! TLS Directory. Other compilers that support TLS and work with the
-//! Microsoft linker must use this same technique.
-//! ```
-//!
-//! Basically what this means is that if we want support for our TLS
-//! destructors/our hook being called then we need to make sure the linker does
-//! not omit this symbol. Otherwise it will omit it and our callback won't be
-//! wired up.
-//!
-//! We don't actually use the `/INCLUDE` linker flag here like the article
-//! mentions because the Rust compiler doesn't propagate linker flags, but
-//! instead we use a shim function which performs a volatile 1-byte load from
-//! the address of the _tls_used symbol to ensure it sticks around.
-//!
-//! [1]: https://www.codeproject.com/Articles/8113/Thread-Local-Storage-The-C-Way
-//! [2]: https://github.com/ChromiumWebApps/chromium/blob/master/base/threading/thread_local_storage_win.cc#L42
+//! [1]: https://devblogs.microsoft.com/oldnewthing/20191011-00/?p=102989
 
 use core::ffi::c_void;
+use core::sync::atomic::{AtomicBool, AtomicU32, Ordering, fence};
 
+use crate::cell::Cell;
 use crate::ptr;
-use crate::sys::c;
+use crate::sys::c::{self, FLS_OUT_OF_INDEXES};
+
+pub type Key = u32;
+
+unsafe fn create(dtor: c::PFLS_CALLBACK_FUNCTION) -> Key {
+    let key_result = unsafe { c::FlsAlloc(dtor) };
+
+    if key_result == c::FLS_OUT_OF_INDEXES {
+        rtabort!("out of FLS keys");
+    }
 
-unsafe extern "C" {
-    #[link_name = "_tls_used"]
-    static TLS_USED: u8;
+    key_result
 }
-pub fn enable() {
-    // When destructors are used, we need to add a reference to the _tls_used
-    // symbol provided by the CRT, otherwise the TLS support code will get
-    // GC'd by the linker and our callback won't be called.
-    unsafe { ptr::from_ref(&TLS_USED).read_volatile() };
-    // We also need to reference CALLBACK to make sure it does not get GC'd
-    // by the compiler/LLVM. The callback will end up inside the TLS
-    // callback array pointed to by _TLS_USED through linker shenanigans,
-    // but as far as the compiler is concerned, it looks like the data is
-    // unused, so we need this hack to prevent it from disappearing.
-    unsafe { ptr::from_ref(&CALLBACK).read_volatile() };
+
+unsafe fn set(key: Key, ptr: *const c_void) {
+    let result = unsafe { c::FlsSetValue(key, ptr) };
+
+    if result == c::FALSE {
+        rtabort!("failed to set FLS value");
+    }
+}
+
+fn is_thread_a_fiber() -> bool {
+    let res = unsafe { c::IsThreadAFiber() };
+    res == c::TRUE
+}
+
+static KEY: AtomicU32 = AtomicU32::new(FLS_OUT_OF_INDEXES);
+
+/// Used to track whether we are currently in the critical section of `enable`.
+/// For miri, these atomic operations cause synchronization that can mask user bugs,
+/// and they are not needed as `atexit` is anyway not supported, so we can skip them.
+struct EnableGuard;
+static AT_EXIT_HOOK_CALLED: AtomicBool = AtomicBool::new(false);
+static ACTIVE_ENABLE_CALLS: AtomicU32 = AtomicU32::new(0);
+
+impl EnableGuard {
+    // Mark the start of an `enable` call, returning whether the `atexit` hook has already been called or not.
+    fn new() -> (Self, bool) {
+        if cfg!(miri) {
+            return (Self, false);
+        }
+        ACTIVE_ENABLE_CALLS.fetch_add(1, Ordering::Relaxed);
+
+        // Both `new` and `start_exit` publish state to one atomic and inspect the other.
+        // `AcqRel` is insufficient because neither read is required to observe the other's publication,
+        // so we could create the guard but `start_exit` would not see any active enable calls.
+        // `SeqCst` ensures that there's a single global order between the publish and check,
+        // so at least one side must observe the other and bail.
+        fence(Ordering::SeqCst);
+
+        let at_exit_called = AT_EXIT_HOOK_CALLED.load(Ordering::Relaxed);
+
+        (Self, at_exit_called)
+    }
+
+    /// Mark the start of process exit, returning whether we should free the FLS key or not.
+    fn start_exit() -> bool {
+        // After this hook starts, new destructor registration will be skipped,
+        // causing TLS destructors initialized after this point to leak.
+        if AT_EXIT_HOOK_CALLED.swap(true, Ordering::Relaxed) {
+            // Cleanup already started, there is nothing else to do.
+            return false;
+        }
+
+        fence(Ordering::SeqCst);
+
+        let any_active_enabled_called = ACTIVE_ENABLE_CALLS.load(Ordering::Relaxed) != 0;
+
+        if any_active_enabled_called {
+            // If another thread is currently in `enable`, it may already have loaded this key and may be about to call `FlsSetValue`.
+            // So we must *not* call free the FLS key.
+            //
+            // During real process exit this is harmless because the `cleanup` hook is always available,
+            // and the FLS callback will be triggered normally by the OS.
+            //
+            // During DLL unload, the unloader cannot safely have threads running code from the DLL except for the destructors,
+            // so there must not be any `enable` calls active anyway.
+            return false;
+        }
+
+        return true;
+    }
 }
 
-#[unsafe(link_section = ".CRT$XLB")]
-#[cfg_attr(miri, used)] // Miri only considers explicitly `#[used]` statics for `lookup_link_section`
-pub static CALLBACK: unsafe extern "system" fn(*mut c_void, u32, *mut c_void) = tls_callback;
+#[cfg(not(miri))]
+impl Drop for EnableGuard {
+    fn drop(&mut self) {
+        ACTIVE_ENABLE_CALLS.fetch_sub(1, Ordering::Relaxed);
+    }
+}
 
-unsafe extern "system" fn tls_callback(_h: *mut c_void, dw_reason: u32, _pv: *mut c_void) {
-    if dw_reason == c::DLL_THREAD_DETACH || dw_reason == c::DLL_PROCESS_DETACH {
-        unsafe {
-            #[cfg(target_thread_local)]
-            super::super::destructors::run();
-            #[cfg(not(target_thread_local))]
-            super::super::key::run_dtors();
+pub fn enable() {
+    let registered = if cfg!(target_thread_local) {
+        #[thread_local]
+        static REGISTERED: Cell<bool> = Cell::new(false);
+        REGISTERED.replace(true)
+    } else {
+        // `#[thread_local]` is unavailable on windows-gnu (`target_thread_local` is off),
+        // but setting the FLS key's value is about as expensive as `TlsGet`, so we don't bother tracking registration separately.
+        false
+    };
 
-            crate::rt::thread_cleanup();
+    if !registered {
+        // We are in a critical section where we are trying to register a destructor for the current thread.
+        // We need to avoid racing with the `atexit` hook that frees the FLS slot, which would cause us to call `FlsSetValue` on a freed key,
+        // or calling `atexit` during process shutdown, which would cause a deadlock.
+        let (_guard, at_exit_called) = EnableGuard::new();
+
+        if at_exit_called {
+            // We are exiting and don't want to race with the `atexit` hook, so we won't be able to run the destructors for this thread.
+            return;
         }
+
+        let current_key = KEY.load(Ordering::Acquire);
+
+        // If we already allocated a key, we only need to set it to a non-null value so that the destructors hook is run for this thread.
+        let key = if current_key != FLS_OUT_OF_INDEXES {
+            current_key
+        } else {
+            // Otherwise, we try to allocate a key.
+            let new_key = unsafe { create(Some(cleanup)) };
+
+            // Now we need to set this key to be used by everyone else.
+            // If we won the race, our key is the right one and we can set it to non-null value.
+            // If we lost, we'll use the winning key and free our losing key.
+            match KEY.compare_exchange(current_key, new_key, Ordering::Release, Ordering::Acquire) {
+                Ok(_) => {
+                    // If the current DLL is unloaded, the registered `cleanup` hook will not be available later during thread exit,
+                    // triggering a `STATUS_ACCESS_VIOLATION`. To avoid this, we use the `atexit` hook, which is called during DLL unload
+                    // to manually free the FLS slot, triggering the destructors.
+                    //
+                    // However, calling `atexit` during process exit can cause a deadlock.
+                    // In a Rust binary, `enable` is called during the main thread startup and before any user code,
+                    // and we checked using `at_exit_called` that we aren't in process shutdown.
+                    //
+                    // In a Rust DLL, dynamic unloading can only happen safely when no other threads are
+                    // concurrently executing Rust code, so if we are here we cannot be unloading yet.
+                    //
+                    // If a main non-Rust binary is exiting, it must not be trigger the `enable` guard
+                    // for the first time during process shutdown.
+                    let res = unsafe { c::atexit(free_fls_key_at_exit) };
+                    if res != 0 {
+                        rtabort!("failed to register fls atexit hook");
+                    }
+
+                    new_key
+                }
+                Err(other_key) => {
+                    unsafe { c::FlsFree(new_key) };
+                    other_key
+                }
+            }
+        };
+
+        // Setting the key's value to non-zero will cause the dtor callback to be called when the thread exits.
+        unsafe { set(key, ptr::without_provenance(1)) };
+    }
+}
+
+extern "C" fn free_fls_key_at_exit() {
+    // The main purpose of this hook is to free the FLS slot during DLL unload.
+    // However, this hook will also be called during normal process exit, while other Rust threads are still running,
+    // so we must be careful to avoid races with `enable`.
+    let should_free_key = EnableGuard::start_exit();
+    if !should_free_key {
+        return;
+    }
+
+    let current_key = KEY.swap(c::FLS_OUT_OF_INDEXES, Ordering::AcqRel);
+    if current_key != c::FLS_OUT_OF_INDEXES {
+        // Calling `FlsFree` will cause the OS to call the `cleanup` hook, in the current thread, *for each thread* (or fiber) with a value in this FLS slot.
+        // `cleanup` is safe to run repeatedly: it only drains the current thread's TLS destructor list, and we check that we are not running in a fiber before doing so.
+        // We only call this when no `enable` call is active, so it cannot race with `FlsSetValue` using this key.
+        // Destructors of thread locals in other threads will not run and therefore leak, which is allowed since we are exiting or unloading.
+        unsafe { c::FlsFree(current_key) };
+    }
+}
+
+unsafe extern "system" fn cleanup(_ptr: *const c_void) {
+    // Avoid running the hook if we are in a fiber.
+    // This will cause destructors of thread locals to not run, leaking them.
+    // Thread-local runtime state will not be cleaned.
+    //
+    // We need to verify that we won't run the destructors *before* the thread exits,
+    // but if the fiber that registered the callback is deleted, the thread might still be running other fibers.
+    //
+    // By checking that we are not running in a fiber here, we are guaranteed that the hook is only running during the thread's exit.
+    // See also the `fiber_does_not_trigger_dtor` test.
+    if is_thread_a_fiber() {
+        return;
     }
+
+    unsafe {
+        #[cfg(target_thread_local)]
+        super::super::destructors::run();
+        #[cfg(not(target_thread_local))]
+        super::super::key::run_dtors();
+    }
+
+    crate::rt::thread_cleanup();
 }