Repository navigation
Miri sometimes reports memory leak in thread-local storage on *-pc-windows-gnu #123583
Description
I've only seen this once, even though the test should be covered by miri-test-libstd every day:
error: memory leaked: alloc26870878 (Rust heap, size: 8, align: 4), allocated here:
Error: --> /checkout/library/alloc/src/alloc.rs:100:9
|
100 | __rust_alloc(layout.size(), layout.align())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: BACKTRACE:
= note: inside `realstd::alloc::alloc` at /checkout/library/alloc/src/alloc.rs:100:9: 100:52
= note: inside `realstd::alloc::Global::alloc_impl` at /checkout/library/alloc/src/alloc.rs:183:73: 183:86
= note: inside `<realstd::alloc::Global as core::alloc::Allocator>::allocate` at /checkout/library/alloc/src/alloc.rs:243:9: 243:39
= note: inside `alloc_crate::alloc::exchange_malloc` at /checkout/library/alloc/src/alloc.rs:332:11: 332:34
= note: inside `realstd::boxed::Box::<realstd::sys::thread_local::os_local::Value<u8>>::new` at /checkout/library/alloc/src/boxed.rs:218:9: 218:20
= note: inside `realstd::thread::local_impl::Key::<u8>::try_initialize::<{closure@/checkout/library/std/src/sys/thread_local/os_local.rs:28:27: 28:34}>` at /checkout/library/std/src/sys/thread_local/os_local.rs:145:37: 145:94
= note: inside `realstd::thread::local_impl::Key::<u8>::get::<{closure@/checkout/library/std/src/sys/thread_local/os_local.rs:28:27: 28:34}>` at /checkout/library/std/src/sys/thread_local/os_local.rs:127:18: 127:43
note: inside `sync::reentrant_lock::current_thread_unique_ptr::X::__getit`
--> /checkout/library/std/src/sys/thread_local/os_local.rs:28:17
|
11 | pub macro thread_local_inner {
| ----------------------------
| |
| in this expansion of `$crate::thread::local_impl::thread_local_inner!` (#2)
| in this expansion of `$crate::thread::local_impl::thread_local_inner!` (#3)
...
28 | / __KEY.get(move || {
29 | | if let $crate::option::Option::Some(init) = _init {
30 | | if let $crate::option::Option::Some(value) = init.take() {
31 | | return value;
... |
36 | | __init()
37 | | })
| |__________________^
...
82 | $crate::thread::local_impl::thread_local_inner!(@key $t, $($init)*);
| ------------------------------------------------------------------- in this macro invocation (#3)
|
::: library/std/src/sync/reentrant_lock.rs:318:5
|
318 | thread_local! { static X: u8 = const { 0 } }
| -------------------------------------------- in this macro invocation (#1)
|
::: library/std/src/thread/local.rs:183:1
|
183 | / macro_rules! thread_local {
184 | | // empty (base case for the recursion)
185 | | () => {};
... |
193 | | $crate::thread::local_impl::thread_local_inner!($(#[$attr])* $vis $name, $t, const $init);
| | ----------------------------------------------------------------------------------------- in this macro invocation (#2)
... |
205 | | );
206 | | }
| |_- in this expansion of `thread_local!` (#1)
= note: inside `realstd::thread::LocalKey::<u8>::try_with::<{closure@library/std/src/sync/reentrant_lock.rs:319:12: 319:15}, usize>` at /checkout/library/std/src/thread/local.rs:283:32: 283:50
= note: inside `realstd::thread::LocalKey::<u8>::with::<{closure@library/std/src/sync/reentrant_lock.rs:319:12: 319:15}, usize>` at /checkout/library/std/src/thread/local.rs:260:9: 260:25
note: inside `sync::reentrant_lock::current_thread_unique_ptr`
--> library/std/src/sync/reentrant_lock.rs:319:5
|
319 | X.with(|x| <*const _>::addr(x))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `sync::reentrant_lock::ReentrantLock::<()>::lock`
--> library/std/src/sync/reentrant_lock.rs:184:27
|
184 | let this_thread = current_thread_unique_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `sync::reentrant_lock::tests::smoke`
--> library/std/src/sync/reentrant_lock/tests.rs:10:17
|
10 | let a = l.lock();
| ^^^^^^^^
note: inside closure
--> library/std/src/sync/reentrant_lock/tests.rs:7:11
|
6 | #[test]
| ------- in this procedural macro expansion
7 | fn smoke() {
| ^
|
::: /checkout/library/core/src/macros/mod.rs:1636:5
|
1636 | pub macro test($item:item) {
| -------------- in this expansion of `#[test]`
Activity
RalfJung commentedon Apr 7, 2024
Or maybe this is caused by Miri not properly running the on-thread-exit hook on Windows? The support for that is a bit fragile as Miri does not support these magic linker sections we rely on. Instead Miri hard-codes the path to this static and calls it. But with the test crate we may have that static twice (once in std and once in the test crate) and only one of them gets called...
However I would expect that this logic means that the thread_local impl from the real std is used, so the one from the test crate should not matter.
Maybe we should stop using this hack and implement support for this magic linker section instead. I just don't know if there's a good way to ask the compiler for "all
staticin a particular link_section across all crates".RalfJung commentedon Apr 7, 2024
I tried to figure out how TLS is currently implemented on Windows, but I got lost in the dark forest that is
std::sys.^^ I can't figure out where this call (imp::create) would go on Windows; nothing in the Windows folder declares acreatefunction.EDIT: Oh, looks like
sys_common::thread_local_keyis Unix-only. I got fooled by the "common" in the name and it not being inpal.ChrisDenton commentedon Apr 7, 2024
The Windows specific bits happen here: https://github.com/rust-lang/rust/blob/e43aef0ef9cc9d2d12c138b36fd817f7c41d0152/library/std/src/sys/pal/windows/thread_local_key.rs
Note though that the
StaticKey(and related) bits aren't actually used if#[cfg(target_thread_local)].RalfJung commentedon Apr 7, 2024
The bug was reported on i686-pc-windows-gnu. Not sure if that's currently
target_thread_localor not... I think it is? Only the 32bit MSVC target is not? But I keep forgetting which one exactly it is so I may be wrong.RalfJung commentedon Apr 7, 2024
However I think even with
target_thread_local, dtors are handled the same way -- viathread::local_impl::Key::<T>::register_dtor.ChrisDenton commentedon Apr 7, 2024
32-bit msvc is currently not
target_thread_local. However, that may change soon.register_keyless_dtoris used whentarget_thread_localis enabled, otherwiseregister_dtoris used. Both ultimately use the special.CRT$XLBlinker magic.RalfJung commentedon Apr 7, 2024
Ah,
register_dtoris actuallyregister_keyless_dtor.This is such a maze...^^
ChrisDenton commentedon Apr 7, 2024
You're not the first to notice that: #110897
[-]Miri sometimes reports memory leak in reentrant lock on Windows[/-][+]Miri sometimes reports memory leak in thread-local storage on Windows[/+]14 remaining items
ChrisDenton commentedon Apr 22, 2024
Presumably this is due to the TLS dtor when not
has_thread_local.All the msvc targets currently use the
#[cfg(target_thread_local)]code.RalfJung commentedon Apr 22, 2024
Meanwhile, #124270 should work around the CI issue.
RalfJung commentedon Apr 22, 2024
FWIW the issue reproduces with and without
const { ... }.RalfJung commentedon Apr 22, 2024
RalfJung commentedon Apr 22, 2024
Oh never mind, I copied the span from the closure not the stacktrace.^^
It's this Box:
rust/library/std/src/sys/thread_local/os_local.rs
Line 145 in 45ca53f
RalfJung commentedon Apr 22, 2024
One key ingredient to the leak seems to be not loading the latest value here:
rust/library/std/src/sys/pal/windows/thread_local_key.rs
Line 314 in 5934aaa
Cc @joboet
Auto merge of rust-lang#124270 - RalfJung:miri-libstd-win, r=onur-ozkan
RalfJung commentedon Apr 23, 2024
Ah, I think I got it. :)
#124281
Rollup merge of rust-lang#124281 - RalfJung:win-tls, r=joboet
Unrolled build for rust-lang#124281
matthiaskrgr commentedon Apr 24, 2024
🏅
Rollup merge of #124281 - RalfJung:win-tls, r=joboet
Auto merge of #3501 - RalfJung:tls-many-seeds, r=RalfJung
Auto merge of rust-lang#3501 - RalfJung:tls-many-seeds, r=RalfJung