Repository navigation
std::process::exit is not thread-safe in combination with C code calling exit #126600
Description
Calling exit concurrently from Rust and C (or from 2 different copies of the Rust runtime in the same binary) is UB. This is permitted by the C standard, and can actually cause trouble in practice since not all versions of all libc implementations do proper locking when traversing the list of atexit functions. For programs only using Rust, we mitigated this by adding a lock, but this only helps when all calls to exit come from Rust (and they must come from the same Rust runtime). Note that returning from main is equivalent to calling exit so it can also cause this issue.
Current status
Mitigation on our end (only covers pure Rust programs): #126606
The current status is that the specification language stems from a time when C did not specify multithreading and the intent was to forbid reentrancy. External discussion indicates that there's intent to fix it both in libc implementations and the specs:
- glibc: https://sourceware.org/bugzilla/show_bug.cgi?id=31997
- POSIX: https://austingroupbugs.net/view.php?id=1845
- also a comment mentioning taking it to the C committee.
- libc-coord (including musl author): https://www.openwall.com/lists/libc-coord/2024/07/24/4
- musl patch: https://git.musl-libc.org/cgit/musl/commit/?id=8cca79a72cccbdb54726125d690d7d0095fc2409
- freebsd patch: https://reviews.freebsd.org/D46108
- bionic: https://android.googlesource.com/platform/bionic.git/+/089f4d17265480a5ad9311bcbd8890bc9f361801%5E%21/
Original bugreport (partially outdated)
The current implementation of std::process::exit is unsound on Linux and possibly related platforms where it defers to libc's exit() function. exit() is documented to be not thread-safe, and hence std::process::exit is not thread-safe either, despite being a non-unsafe function.
To show that this isn't just a theoretical problem, here is a minimal example that segfaults on my machine (Ubuntu with glibc 2.37):
use std::thread;
fn main() {
for _ in 0..32 {
unsafe { libc::atexit(exit_handler) };
}
for _ in 0..2 {
thread::spawn(|| std::process::exit(0));
}
}
extern "C" fn exit_handler() {
thread::sleep_ms(1000);
}The example contains unsafe code, but only to install exit handlers. AFAICT nothing about the libc::atexit call is unsafe. The UB is introduced by calling std::process::exit concurrently afterwards.
If you are curious, https://github.com/MaterializeInc/database-issues/issues/6528 lays out what causes the segfault (it's a use-after-free). That's not terribly relevant though, given that glibc has no obligation to ensure exit() is thread safe when it's clearly documented not to be. Instead, Rust should either mark std::process::exit as unsafe (which is something it could do for the 2024 edition), or introduce locking to ensure only a single thread gets to call exit() at a time.
Activity
teskje commentedon Jun 17, 2024
This has been discussed previously in #83994. There the decision was to not do anything about the concerns with
std::process::exit, mostly based on the reasoning thatexit()is thread safe. The above example shows it's clearly not, neither according to the documentation nor in the implementation.Given that I just spend the better part of a weekend debugging a segfault that was caused by concurrent
std::process::exitinvocations I feel quite strongly that this issue deserves reconsideration.ChrisDenton commentedon Jun 17, 2024
On that thread joshtriplett notes:
We do now mark
std::env::set_varas unsafe so maybe thinking on this has changed. Though I'm not sure how you would mark returning from main as beingunsafe?conradludgate commentedon Jun 17, 2024
Linux libc documentation does claim that
exit()is thread-unsafe: "MT-Unsafe race:exit", although the original thread suggests that all libc implementations introduce appropriate locks. Perhaps introducing aOncebefore calling libc::exit is sufficient and not a performance blocker?the8472 commentedon Jun 17, 2024
Doesn't solve the problem with exit getting called from non-rust code. And no, an atexit handler won't help since it leaves a race-window and if you're in a situation where threads concurrently call exit you're already racing.
https://github.com/MaterializeInc/database-issues/issues/6528 sounds like it's either a libc bug or the assessment from the previous thread that libc implementations provide the desired behavior - even if the standard language doesn't - needs to be revised.
ChrisDenton commentedon Jun 17, 2024
I'd guess the safe thing to do is kill all other threads then call
exit.the8472 commentedon Jun 17, 2024
Not sure if serious...
Killing threads would make anything that accesses their stacks UB. Freezing them would work but that's difficult to implement reliably. Libc would be in a position to do it since they control pthreads. But then they might as well fix their locking.
tbu- commentedon Jun 17, 2024
Adding 32
atexithandlers seems to be essential to make this segfault on my glibc 2.39+r52+gf8e4623421-1 on Arch Linux. With 31atexithandlers, it doesn't segfault.tbu- commentedon Jun 17, 2024
I'd say it makes sense to introduce a lock on the Rust side, since pure-Rust code shouldn't be able to cause UB by calling libc's
exiton different threads.This also means that Rust cdylibs mustn't call
std::process::exitsince they might not be the only language running, and applications must ensure that no foreign functions callexit. The latter should be a bug anyway due to the C standard saying that multiple calls toexitare UB.teskje commentedon Jun 17, 2024
Based on how the glibc code is set up it very much looks like
exitisn't intended to be thread safe. It uses a lock, but releases it every time it calls one of the exit handlers. That'd make no sense if it wanted to prevent other threads from modifying the list of exit handlers concurrently. The lock is useful for preventing corruption in the face of other threads callingatexitin parallel. There is a comment that says that this is its intended purpose (though admittedly it could be clearer).I think the previous assumption that glibc's
exitis thread-safe was a misconception based on the existence of this lock alone.teskje commentedon Jun 17, 2024
That's explained by the fact that one block in the list of exit handlers contains 32 entries, and the last block is never freed. So to trigger the use-after-free you need at least two blocks in the list. glibc installs one exit handler on its own, so 32 more need to be installed to add a free-able block to the list.
152 remaining items
bstrie commentedon Oct 23, 2024
This is a rather long thread, can we get a summary of the current status added to the original issue body? What are actionable steps that can be taken to resolve this unsound issue? AFAICT the status appears to be "wait until the rest of the world is fixed", which IMO is tantamount to closing this issue as WONTFIX, given the prospects of that ever happening. And I'm not entirely satisfied by the dismissal of ameliorations on the Rust side (either adding a lock or marking as
unsafe). If we can do better, then let's do better, even if preventing people from linking to broken C code is out of our power.ChrisDenton commentedon Oct 23, 2024
The pure Rust fix would be to have some Rust API for adding on exit functions. This could side-step the issue entirely so long as you ignore C.
Note that we already have a lock to mitigate this issue. However it's by no means a fix.
the8472 commentedon Oct 23, 2024
Things are looking rosier than that. Posix, musl and glibc agreed to improve things and if I read the comments on the posix issue correctly they'll also bring it to the appropriate C committee.
the8472 commentedon Oct 23, 2024
I have gathered a short summary and a few links from other comments into the top comment. Edit away if there's anything to correct.
[-]`std::process::exit` is not thread-safe[/-][+]`std::process::exit` is not thread-safe in combination with C code calling `exit`[/+]safinaskar commentedon Jan 29, 2025
Please, document in https://doc.rust-lang.org/nightly/std/process/fn.exit.html , on which platforms
exitis fully thread-safe (even if some C code calls it), and on which platforms it is not. As well as I understand,exitis fully safe on musl. This should be documented on that page.Also, as well as I understand, musl is shipped with Rust itself. This means nothing prevents us from patching it! So, I suggest patching all remaining similarly unsafe C functions (
setenv?) in musl implementation we ship. This will allow us fully solve all these remaining Rust unsoundness problems caused by system libc once and for all (at least for system libraries we ship).Also, we can create alternative target with static glibc. We will patch that glibc however we want, we will fix all
exit,setenv, etc problems there. (Unfortunately, if we really want static glibc, we should build it with--enable-static-nss, but this is blocked by https://sourceware.org/bugzilla/show_bug.cgi?id=27959 )RalfJung commentedon Jan 29, 2025
That is really something the respective platforms need to document. We shouldn't be in the business of providing a central repository of all the ways in which various C implementations promise more than the C standard. We track this for our own needs here, but that is not the same as making it part of the official docs.
And we are also not interested in maintaining a patched fork of musl or glibc.
RalfJung commentedon Jan 29, 2025
What we should do, however, is document that it is UB to call this
exitconcurrently with C code callingexit.Rollup merge of rust-lang#136320 - RalfJung:exit, r=the8472
Rollup merge of rust-lang#136320 - RalfJung:exit, r=the8472
Rollup merge of rust-lang#136320 - RalfJung:exit, r=the8472
Rollup merge of rust-lang#136320 - RalfJung:exit, r=the8472
stdlib: Allow concurrent exit (BZ 31997)
stdlib: Allow concurrent exit (BZ 31997)