Repository navigation
timeout: don't dump core when re-raising the child's signal - #15157
TanbirRamim wants to merge 2 commits into
Conversation
| fn disable_core_dumps() -> bool { | ||
| #[cfg(any(target_os = "linux", target_os = "android"))] | ||
| // SAFETY: PR_SET_DUMPABLE only takes an integer argument. | ||
| if unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0) } == 0 { |
There was a problem hiding this comment.
Would you remove unsafe by rustix?
There was a problem hiding this comment.
Done, it uses rustix now (set_dumpable_behavior + setrlimit), no unsafe left.
| /// When the child dumps core, timeout re-raises its signal without dumping | ||
| /// core itself. | ||
| #[test] | ||
| #[cfg(any(target_os = "linux", target_os = "macos"))] |
There was a problem hiding this comment.
on macOS the child sh will write a real core into /cores, which can be several GB on the runner. Could we keep this test linux only?
There was a problem hiding this comment.
Good point, made it Linux only.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
Binary size comparison: |
Merging this PR will not alter performance1 warning
Comparing Footnotes
|
df98adb to
9dc7673
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
When the child dies from something like SIGSEGV, timeout kills itself with the same signal to pass the status on, and that made timeout dump core too (so apport blamed timeout for the crash). Now it turns off core dumps first like GNU does (prctl PR_SET_DUMPABLE, falling back to RLIMIT_CORE=0) and only re-raises if that worked.
Added a test that runs it under
ulimit -c unlimitedand checks timeout's own status isn't a core dump. It fails on Linux without the fix. I didn't add the "monitored command dumped core" message, can do that in a follow-up if wanted.Fixes #15144