Inappropriate `panic!`s in `Thread::new` on certain unix platforms
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 119k
- Forks
- 16.1k
- PR merge metrics
- PR metrics pending
Description
Found after discussing code surrounding #160219 on zulip.
Edit: It is inappropriate for Thread::new to ever panic, because we guarantee that thread::current, which uses it, does not call the global allocator reentrantly. The memory leak on unwind, which this issue was originally about, is not that important.
The implementation of Thread::new manually initializes a struct behind MaybeUninit, which means that if Parker::new_in_place unwinds, the name's memory (essentially Option<CString>), now held by the previously written name field behind MaybeUninit, is leaked.
Note that this is possible, on certain unix platforms:
which calls
which uses assert_eq to ensure that the initialization of the Condvar succeeds (it can fail, e.g. from resource exhaustion). Note that the method itself expects failures are possible here. It uses a guard struct to ensure that the resources of the condattr are not leaked on unwind.
Furthermore, the implementation of std::thread::current seems to assume that Thread::new never unwinds, because it sets a flag to detect reentrance, which is never reset if an unwind occurs:
The first issue should be fixable by reordering the fields' initialization logic, and the second one by adding a drop guard that unsets the busy state.
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
Read library/std/src/thread/thread.rs around Thread::new, then trace Parker::new_in_place through library/std/src/sys/sync/thread_parking/pthread.rs and library/std/src/sys/pal/unix/sync/condvar.rs. Also inspect library/std/src/thread/current.rs around the reentrance flag; done means Thread::new does not panic in the described failure paths and current's state remains correct if unwinding occurs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100