ore: make halt! use _exit to avoid exit handler races
#27705
Conversation
teskje
changed the title
halt! use _exit to avoid exit handler races
|
"Longer Zippy ClusterReplicas" still fails, but not with a segfault anymore. Instead it seems to run into a thread limit, which likely unrelated. |
| /// | ||
| /// This function exists to avoid that all callers of `halt!` have to explicitly depend on `libc`. | ||
| pub fn halt() -> ! { | ||
| unsafe { libc::_exit(2) }; |
There was a problem hiding this comment.
TODO(@teskje) some documentation about the use of _exit here
teskje
requested review from
aljoscha,
danhhz,
jkosh44 and
a team
as code owners
|
@MaterializeInc/testing The second PR re-enables the "Longer Zippy ClusterReplicas" test in the Release Qualification pipeline. Given that it failed once with an unrelated error, should I drop that commit? If I do the "references to already closed issues" check might start triggering on this. |
Go ahead, feel free to re-enable that check. We can disable it again if we see that further causes make it fail. |
This PR fixes a soundness hole introduced by multiple threads calling
halt!at the same time, thereby invoking the thread-unsafe libcexitfunction concurrently, which would lead to use-after-frees and resulting segfaults. The fix is to call_exitinstead, which is thread-safe.We can consider reverting this once rust-lang/rust#126600 is resolved.
Motivation
Fixes MaterializeInc/database-issues#6528
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.