diff --git a/library/std/src/sys/thread_local/guard/windows.rs b/library/std/src/sys/thread_local/guard/windows.rs index d59631d5d6ca3..212e8ccdc9d60 100644 --- a/library/std/src/sys/thread_local/guard/windows.rs +++ b/library/std/src/sys/thread_local/guard/windows.rs @@ -176,6 +176,12 @@ pub fn enable() { } }; + // We must not set the key if we are in a fiber, since deleting that fiber from a thread + // will cause the destructors to run before thread exit. + if is_thread_a_fiber() { + return; + } + // 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)) }; } diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 7a05a962e2ac0..18b0f3263ad59 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -98,17 +98,16 @@ use crate::fmt; /// run on the thread that causes the process to exit. This is because the /// other threads may be forcibly terminated. /// -/// If a thread is [converted into a fiber], destructors will not be run unless -/// the fiber is [converted back into a thread] before the underlying thread exits. +/// TLS destructors may be leaked if a thread exits while [converted into a fiber], +/// or if Rust TLS destructor support is first needed while running in a fiber. /// /// If a process loads a Rust `cdylib`, it must not cause the Rust TLS destructor support -// to be initialized for the first time during process shutdown. +/// to be initialized for the first time during process shutdown. /// /// When dynamically unloading a Rust `cdylib`, pending TLS destructors may run -// during the unload or may be leaked. +/// during the unload or may be leaked. /// /// [converted into a fiber]: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-convertthreadtofiber -/// [converted back into a thread]: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-convertfibertothread /// [loader lock]: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-best-practices /// [`with`]: LocalKey::with #[cfg_attr(not(test), rustc_diagnostic_item = "LocalKey")] diff --git a/library/std/tests/thread_local/tests.rs b/library/std/tests/thread_local/tests.rs index 1a25d91e43bc0..df9b99ef0af64 100644 --- a/library/std/tests/thread_local/tests.rs +++ b/library/std/tests/thread_local/tests.rs @@ -416,9 +416,17 @@ fn fiber_does_not_trigger_dtor() { unsafe extern "system" { fn ConvertFiberToThread() -> i32; fn ConvertThreadToFiber(lpParameter: *const c_void) -> *mut c_void; + fn CreateFiber( + dwStackSize: usize, + lpStartAddress: unsafe extern "system" fn(*mut c_void), + lpParameter: *mut c_void, + ) -> *mut c_void; + fn DeleteFiber(lpFiber: *mut c_void); + fn SwitchToFiber(lpFiber: *mut c_void); } thread_local!(static FOO: UnsafeCell> = UnsafeCell::new(None)); + let signal = Signal::default(); let signal2 = signal.clone(); @@ -438,13 +446,49 @@ fn fiber_does_not_trigger_dtor() { // As long as we stop using fibers before thread teardown, everything works as expected. let signal2 = signal.clone(); let t = thread::spawn(move || unsafe { - let mut signal = Some(signal2); - let _ = ConvertThreadToFiber(ptr::null()); - FOO.with(|f| { - *f.get() = Some(NotifyOnDrop(signal.take().unwrap())); - }); - let _ = ConvertFiberToThread(); + struct FiberData { + main: *mut c_void, + signal: Signal, + } + + unsafe extern "system" fn fiber_start(data: *mut c_void) { + let data = unsafe { &mut *data.cast::() }; + + // Set the value while this fiber is current. + // This must NOT arm the FLS cleanup guard for the fiber. + FOO.with(|f| unsafe { + *f.get() = Some(NotifyOnDrop(data.signal.clone())); + }); + + unsafe { + SwitchToFiber(data.main); + } + } + + let main = ConvertThreadToFiber(ptr::null()); + assert!(!main.is_null()); + + let mut data = FiberData { main, signal: signal2.clone() }; + let foo = CreateFiber(0, fiber_start, ptr::from_mut(&mut data).cast()); + assert!(!foo.is_null()); + + // Run `foo`, which sets FOO while `foo` is the current fiber, + // then switches back to main. + SwitchToFiber(foo); + + // Convert main back to a thread before deleting `foo`. + assert_ne!(ConvertFiberToThread(), 0); + + // Deleting `foo` must not trigger dtors like a thread teardown. + DeleteFiber(foo); + assert!(!signal2.is_set()); + + // Arm the guard now from the normal thread. + // `FOO`'s destructor is already registered, so it will run when the thread exits. + thread_local!(static BAR: UnsafeCell> = UnsafeCell::new(None)); + BAR.with(|_| {}); }); + signal.wait(); assert!(signal.is_set()); t.join().unwrap();