Skip to content

daemonize: use an allow-list approach to inherited FDs - #2841

Merged
sylvestre merged 1 commit into
mozilla:mainfrom
schopin-mozilla:close-inherited-fds-on-daemonize
Sep 11, 2026
Merged

sylvestre merged 1 commit into
mozilla:mainfrom
schopin-mozilla:close-inherited-fds-on-daemonize

Conversation

@schopin-mozilla

Copy link
Copy Markdown
Contributor

Our current approach is to try and close the jobserver file descriptors inherited from make, using heuristics based on parsing the MAKEFLAGS searching for explicit FD numbers. However, in some circumstances, make will dup() those file descriptors internally, and we'll inherit and leak those duplicated FDs, which can lead to build script deadlocking.

This happens to me frequently when working on Firefox on my macOS machine: when I run ./mach lint --outgoing with substantial Rust work, once the clippy run ends, make idles waiting for job tokens to be handed back through those open file descriptors, until the daemonized server finally finishes through its idle timeout, closing their end of the pipes.

So, instead of trying to pinpoint what must absolutely be closed, we take the opposite approach, and assume that most file descriptors don't need to be inherited when spawning a daemon.

Our current approach is to try and close the jobserver file descriptors
inherited from make, using heuristics based on parsing the MAKEFLAGS
searching for explicit FD numbers. However, in some circumstances, make
will dup() those file descriptors internally, and we'll inherit and leak
those duplicated FDs, which can lead to build script deadlocking.

This happens to me frequently when working on Firefox on my macOS
machine: when I run `./mach lint --outgoing` with substantial Rust work,
once the clippy run ends, `make` idles waiting for job tokens to be
handed back through those open file descriptors, until the daemonized
server finally finishes through its idle timeout, closing their end of
the pipes.

So, instead of trying to pinpoint what must absolutely be closed, we
take the opposite approach, and assume that most file descriptors don't
need to be inherited when spawning a daemon.
@codecov-commenter

codecov-commenter commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.62500% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.91%. Comparing base (05aafc8) to head (10cb7cb).

Files with missing lines Patch % Lines
src/util.rs 66.66% 9 Missing ⚠️
src/bin/sccache-dist/main.rs 0.00% 1 Missing ⚠️
src/commands.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2841      +/-   ##
==========================================
+ Coverage   73.84%   75.91%   +2.06%     
==========================================
  Files          72       72              
  Lines       38178    39384    +1206     
==========================================
+ Hits        28194    29899    +1705     
+ Misses       9984     9485     -499     

☔ 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.

@sylvestre
sylvestre merged commit be297ee into mozilla:main Sep 11, 2026
51 checks passed
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.

3 participants