Skip to content

server: don't panic when a finished compile has neither code nor signal - #2776

Open
jetm wants to merge 1 commit into
mozilla:mainfrom
jetm:dist-server-signalless-panic
Open

server: don't panic when a finished compile has neither code nor signal#2776
jetm wants to merge 1 commit into
mozilla:mainfrom
jetm:dist-server-signalless-panic

Conversation

@jetm

@jetm jetm commented Jul 21, 2026

Copy link
Copy Markdown

Part of #2775 (splitting the OpenEmbedded/Yocto sccache-dist work into focused
PRs; replaces the closed #2750).

Problem

get_signal did status.signal().expect("must have signal"), assuming the Unix
invariant that an ExitStatus with no exit code was terminated by a signal. That
does not always hold: an ExitStatus reconstructed for a distributed compile (or
an abnormal wait status such as WIFSTOPPED) can report neither a code nor a
signal. When that happened the expect() panicked the compile task, which the
server surfaced as a misleading "Failed to bind socket" and, under load,
repeatedly fell back to local compilation. Found under a Yocto/BitBake host-mode
distributed build.

Approach

Return Option<i32> from get_signal and assign it straight into res.signal,
so a compile that reports neither code nor signal leaves res.signal unset
instead of crashing the in-flight task. The Windows arm returns None rather
than panicking; ExitStatus::code() is always Some there, so the signal branch
is never reached anyway.

Test

Adds a unit test covering a real terminating signal (SIGKILL) and the
neither-code-nor-signal case (WIFSTOPPED via from_raw), which previously
panicked.

get_signal did `status.signal().expect("must have signal")`, assuming the
Unix invariant that an ExitStatus with no exit code was terminated by a
signal. That does not always hold: an ExitStatus reconstructed for a
distributed compile (or an abnormal wait status such as WIFSTOPPED) can
report neither a code nor a signal. When that happened the expect() panicked
the compile task, which the server surfaced as a misleading "Failed to bind
socket" and, under load, repeatedly fell back to local compilation.

Return Option<i32> from get_signal and assign it straight into res.signal, so
a compile that reports neither code nor signal leaves res.signal unset
instead of crashing the in-flight task. The Windows arm returns None rather
than panicking; ExitStatus::code() is always Some there, so the signal branch
is never reached anyway.

Add a unit test covering a real terminating signal (SIGKILL) and the
neither-code-nor-signal case (WIFSTOPPED via from_raw), which previously
panicked.

Signed-off-by: Javier Tia <javier@peridio.com>
(cherry picked from commit b2c46f6a2f48dbae8ac24ec5eefacac7dc124db2)
@codecov-commenter

codecov-commenter commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 52.63158% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.44%. Comparing base (7cd4f2c) to head (32fc12f).

Files with missing lines Patch % Lines
src/server.rs 52.63% 9 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (7cd4f2c) and HEAD (32fc12f). Click for more details.

HEAD has 5 uploads less than BASE
Flag BASE (7cd4f2c) HEAD (32fc12f)
13 8
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2776      +/-   ##
==========================================
- Coverage   70.90%   62.44%   -8.47%     
==========================================
  Files          71       71              
  Lines       40393    35911    -4482     
==========================================
- Hits        28642    22425    -6217     
- Misses      11751    13486    +1735     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Jul 28, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 45.99%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 10 regressed benchmarks
✅ 48 untouched benchmarks
⏩ 6 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory lru_sequential_scan_pattern 128 B 260 B -50.77%
Memory lru_hotcold_access_pattern 128 B 260 B -50.77%
Memory hash_multiple_files 128 B 260 B -50.77%
Memory time_macro_finder_chunked 128 B 260 B -50.77%
Memory lru_cache_get_hit 128 B 260 B -50.77%
Memory hash_large_data 192 B 324 B -40.74%
Memory hash_header_file 192 B 324 B -40.74%
Memory cache_key_generation 192 B 324 B -40.74%
Memory time_macro_finder_with_macros 192 B 324 B -40.74%
Memory time_macro_finder_no_macros 192 B 324 B -40.74%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing jetm:dist-server-signalless-panic (32fc12f) with main (7cd4f2c)

Open in CodSpeed

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants