Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sccache could be more zealous closing file descriptors when daemonizing #2313

Open
ntrrgc opened this issue Jan 8, 2025 · 2 comments · May be fixed by #2314
Open

sccache could be more zealous closing file descriptors when daemonizing #2313

ntrrgc opened this issue Jan 8, 2025 · 2 comments · May be fixed by #2314

Comments

@ntrrgc
Copy link

ntrrgc commented Jan 8, 2025

Since sccache will be invoked as a generic compiler wrapper, in some situations it might be invoked with pretty exotic file descriptors. For instance, nobody stops me from doing this in a Bash shell, even if it's somewhat questionable:

$ exec 4<> /tmp/a.c  # Open /tmp/a.c as file descriptor number 4 in Bash
$ sccache /usr/bin/gcc -o /tmp/executable -x c /dev/fd/4  # sccache will inherit fd 4, so this will work
$ exec 4>&-  # Close file descriptor 4 in Bash

One of the tasks of the compiler wrapper is to spawn a sccache server when one is not already running. When starting a background process we want as little baggage from the original process as possible, and extra file descriptors are an instance of such baggage that can lead to unexpected problems.

A notorious example of such problems is that ninja will get stuck if executing the sccache wrapper in a tty, because ninja uses a covert pipe to detect when its subprocesses finish execution when sharing a console (!): ninja-build/ninja#2444

You can probably imagine other scenarios in which the daemon accidentally sharing some file descriptors may cause problems.

daemonize (the crate used for most of the setup of spawning the server process) unfortunately does not handle this except for the standard discriptors (stdin, stdout, stderr), and it couldn't without breaking compatibility as a daemon may still need to share specific file descriptors in specific situations.

Nowadays you can use something like close_range() (FreeBSD. Linux 5.9, glibc 2.34). Other approaches involve using /proc/<PID>/fd or similar mechanisms to figure out and close extraneous file descriptors.

@sylvestre
Copy link
Collaborator

which version of sccache did you try ?

We made progress here
#2296
https://bugzilla.mozilla.org/show_bug.cgi?id=1499382

@ntrrgc
Copy link
Author

ntrrgc commented Jan 8, 2025

I tried this on the current top of tree (9ca8beb).

It's reproducible very often with meson projects that end up invoking the compiler wrapper.

$ sccache --stop-server
$ touch meson.build && ninja -C builddir/

ntrrgc added a commit to ntrrgc/sccache that referenced this issue Jan 8, 2025
Otherwise, when the compiler wrapper spawns the sccache server, the
server may end up with unintended file descriptors, which can lead to
unexpected problems.

This is particularly problematic if any of those file descriptors that
accidentally end up in the server process is a pipe, as the pipe will
only be closed when all the processes with that file descriptor close it
or exit.

This was causing sccache to hang ninja, as ninja uses the EOF of a pipe
given to the subprocess to detect when that subprocess has exited:
ninja-build/ninja#2444 (comment)

This patch adds a dependency on the
[close_fds](https://crates.io/crates/close_fds) crate, which
automatically chooses an appropriate mechanism to close a range of file
descriptors. On Linux 5.9+ that mechanism will be libc::close_range().

Fixes mozilla#2313
@ntrrgc ntrrgc linked a pull request Jan 8, 2025 that will close this issue
ntrrgc added a commit to ntrrgc/sccache that referenced this issue Jan 9, 2025
Otherwise, when the compiler wrapper spawns the sccache server, the
server may end up with unintended file descriptors, which can lead to
unexpected problems.

This is particularly problematic if any of those file descriptors that
accidentally end up in the server process is a pipe, as the pipe will
only be closed when all the processes with that file descriptor close it
or exit.

This was causing sccache to hang ninja, as ninja uses the EOF of a pipe
given to the subprocess to detect when that subprocess has exited:
ninja-build/ninja#2444 (comment)

This patch adds a dependency on the
[close_fds](https://crates.io/crates/close_fds) crate, which
automatically chooses an appropriate mechanism to close a range of file
descriptors. On Linux 5.9+ that mechanism will be libc::close_range().

Fixes mozilla#2313
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 a pull request may close this issue.

2 participants