std `stack_overflow` QOI benefits do not seem to justify current complexity
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 119k
- Forks
- 16.1k
- PR merge metrics
- PR metrics pending
Description
I have encountered this issue with rust 1.89.
It was NOT present with rust 1.86
This concerns the file: sys/pal/unix/stack_overflow/thread_info.rs
It contains a static BTreeMap which grows for active threads and shrinks again.
There is a global cleanup handler which gets called before a call to exit()
or by the runtime after main() returns and before the program exits.
Notably the main thread also inserts an element into this map which allocates some internal data structure.
The cleanup handler if all threads have properly terminated will leave the map empty. map.len() == 0.
However the cleanup handler does not get rid of the initial allocation for the now empty map.
Any rust program that uses std will trigger this.
fn main() {
println!("Hello World!");
}
Is enough and will trigger this.
This will trip valgrind on a full leak check (please read on I am aware of the implications of this), because this allocation of the BTreeMap is still reachable.
Valgrind output with build std:
==99723== at 0x48417B4: malloc (vg_replace_malloc.c:381)
==99723== by 0x29942A: std::sys::alloc::unix::<impl core::alloc::global::GlobalAlloc for std::alloc::System>::alloc (unix.rs:14)
==99723== by 0x1DECE9: __rustc::__rdl_alloc (alloc.rs:402)
==99723== by 0x277638: alloc::alloc::alloc (alloc.rs:95)
==99723== by 0x2776C3: alloc::alloc::Global::alloc_impl (alloc.rs:190)
==99723== by 0x279118: <alloc::alloc::Global as core::alloc::Allocator>::allocate (alloc.rs:251)
==99723== by 0x27823F: alloc::boxed::Box<T,A>::try_new_uninit_in (boxed.rs:508)
==99723== by 0x278164: alloc::boxed::Box<T,A>::new_uninit_in (boxed.rs:474)
==99723== by 0x1CFCDE: alloc::collections::btree::node::LeafNode<K,V>::new (node.rs:83)
==99723== by 0x1C8469: alloc::collections::btree::node::NodeRef<alloc::collections::btree::node::marker::Owned,K,V,alloc::collections::btree::node::marker::Leaf>::new_leaf (node.rs:217)
==99723== by 0x216BEC: alloc::collections::btree::map::entry::VacantEntry<K,V,A>::insert_entry (entry.rs:400)
==99723== by 0x217074: alloc::collections::btree::map::entry::VacantEntry<K,V,A>::insert (entry.rs:373)
==99723== LEAK SUMMARY:
==99723== definitely lost: 0 bytes in 0 blocks
==99723== indirectly lost: 0 bytes in 0 blocks
==99723== possibly lost: 0 bytes in 0 blocks
==99723== still reachable: 456 bytes in 1 blocks
==99723== suppressed: 0 bytes in 0 blocks
Valgrind output without build std:
==104772== at 0x48417B4: malloc (vg_replace_malloc.c:381)
==104772== by 0x14AAE6: alloc (alloc.rs:95)
==104772== by 0x14AAE6: alloc_impl (alloc.rs:190)
==104772== by 0x14AAE6: allocate (alloc.rs:251)
==104772== by 0x14AAE6: try_new_uninit_in<alloc::collections::btree::node::LeafNode<usize, std::sys::pal::unix::stack_overflow::thread_info::ThreadInfo>, alloc::alloc::Global> (boxed.rs:508)
==104772== by 0x14AAE6: new_uninit_in<alloc::collections::btree::node::LeafNode<usize, std::sys::pal::unix::stack_overflow::thread_info::ThreadInfo>, alloc::alloc::Global> (boxed.rs:474)
==104772== by 0x14AAE6: new<usize, std::sys::pal::unix::stack_overflow::thread_info::ThreadInfo, alloc::alloc::Global> (node.rs:83)
==104772== by 0x14AAE6: new_leaf<usize, std::sys::pal::unix::stack_overflow::thread_info::ThreadInfo, alloc::alloc::Global> (node.rs:217)
==104772== by 0x14AAE6: insert_entry<usize, std::sys::pal::unix::stack_overflow::thread_info::ThreadInfo, alloc::alloc::Global> (entry.rs:400)
==104772== by 0x14AAE6: insert<usize, std::sys::pal::unix::stack_overflow::thread_info::ThreadInfo, alloc::alloc::Global> (entry.rs:373)
==104772== by 0x14AAE6: insert<usize, std::sys::pal::unix::stack_overflow::thread_info::ThreadInfo, alloc::alloc::Global> (map.rs:1046)
==104772== by 0x14AAE6: std::sys::pal::unix::stack_overflow::thread_info::set_current_info (thread_info.rs:118)
==104772== by 0x14932B: init (stack_overflow.rs:166)
==104772== by 0x14932B: init (mod.rs:48)
==104772== by 0x14932B: init (rt.rs:114)
==104772== by 0x14932B: {closure#0} (rt.rs:173)
==104772== by 0x14932B: do_call<std::rt::lang_start_internal::{closure_env#0}, isize> (panicking.rs:590)
==104772== by 0x14932B: catch_unwind<isize, std::rt::lang_start_internal::{closure_env#0}> (panicking.rs:553)
==104772== by 0x14932B: catch_unwind<std::rt::lang_start_internal::{closure_env#0}, isize> (panic.rs:359)
==104772== by 0x14932B: std::rt::lang_start_internal (rt.rs:171)
==104772== by 0x138EE6: std::rt::lang_start (rt.rs:205)
==104772== by 0x138F8D: main (in REDACTED)
==104772==
==104772== LEAK SUMMARY:
==104772== definitely lost: 0 bytes in 0 blocks
==104772== indirectly lost: 0 bytes in 0 blocks
==104772== possibly lost: 0 bytes in 0 blocks
==104772== still reachable: 456 bytes in 1 blocks
==104772== suppressed: 0 bytes in 0 blocks
I am aware that rust does not wish to fix full leak check findings for complicated situations.
However here the fix appears to be trivial so In my opinion it is reasonable to fix it here.
My fix:
I have changed the function delete_current_info in thread_info.rs
which looks like this in the release:
pub fn delete_current_info() {
let this = errno_location().addr();
let _lock_guard = LOCK.lock();
let _spin_guard = spin_lock_in_setup(this);
// SAFETY: we own the spin lock, so `THREAD_INFO` cannot not be aliased.
let thread_info = unsafe { &mut *(&raw mut THREAD_INFO) };
thread_info.remove(&this);
}
To this:
pub fn delete_current_info() {
let this = errno_location().addr();
let _lock_guard = LOCK.lock();
let _spin_guard = spin_lock_in_setup(this);
// SAFETY: we own the spin lock, so `THREAD_INFO` cannot not be aliased.
let thread_info = unsafe { &mut *(&raw mut THREAD_INFO) };
thread_info.remove(&this);
if thread_info.len() == 0 {
*thread_info = BTreeMap::new();
}
}
This solves the problem and hello world no longer trips valgrind on a full leak check.
I think this fix is probably always safe?
Since for as long as any thread is alive thread_info.len() is never 0.
And once it reaches 0 it is never increased again, but even if that happens by something that I have missed,
then the allocation simply happens again, just like it would if the map were inserted into normally the first time
set_current_info is called when the program starts.
I don't see any downsides to this suggested fix and would therefore appreciate it if it or something similar could be implemented as I have to verify that my programs do not leak any memory including still reachable statics.
This would help me a lot.
Sincerely
Alexander Schütz
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in sys/pal/unix/stack_overflow/thread_info.rs, read delete_current_info and the related set_current_info path. Reproduce the reported Hello World case with a full Valgrind leak check, then verify that the empty THREAD_INFO map no longer leaves its allocation reachable without affecting active-thread cleanup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100