Skip to content

Don't close jobserver fd when executing external subcommand #10477

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

Closed
trinity-1686a opened this issue Mar 14, 2022 · 3 comments · Fixed by #10511
Closed

Don't close jobserver fd when executing external subcommand #10477

trinity-1686a opened this issue Mar 14, 2022 · 3 comments · Fixed by #10511
Labels
A-custom-subcommands Area: custom 3rd party subcommand plugins A-jobserver Area: jobserver, concurrency, parallelism C-bug Category: bug

Comments

@trinity-1686a
Copy link

trinity-1686a commented Mar 14, 2022

Problem

I tried to use cargo-fuzz from a Makefile, expecting it to use make's jobserver to limit concurency.
When running it as cargo-fuzz, it worked as intended, but when running it as cargo fuzz, it did not seem to acknowledge make job count.
I found cargo-fuzz environment variables contained --jobserver-auth=3,4 as expected, but these file descriptors are closed.

Steps

  • create a script called cargo-sleep and put it into your $PATH
#!/bin/bash
sleep $2
  • write a simple Makefile calling this command with jobserver passing
all:
        +cargo sleep 10
  • run the makefile with jobserver
make -j2
  • look at the environment of sleep. It contains MAKEFLAGS= -j2 --jobserver-auth=3,4 (actual number may vary)
  • look at open file descriptors of sleep, 3 and 4 (in my case) are not open.

Possible Solution(s)

No response

Notes

As stated there is a simple workaround, calling the sub-command directly instead of through cargo

Version

cargo 1.59.0 (49d8809dc 2022-02-10)
release: 1.59.0
commit-hash: 49d8809dc2d3e6e0d5ec634fcf26d8e2aab67130
commit-date: 2022-02-10
host: x86_64-unknown-linux-gnu
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1l)
os: Arch Linux [64-bit]
@trinity-1686a trinity-1686a added the C-bug Category: bug label Mar 14, 2022
@weihanglo weihanglo added A-custom-subcommands Area: custom 3rd party subcommand plugins A-jobserver Area: jobserver, concurrency, parallelism labels Mar 22, 2022
@weihanglo
Copy link
Member

weihanglo commented Mar 23, 2022

Thanks for the report. The behaviour indeed reflects how Cargo runs external subcommands: Cargo doesn't call ProcessBuilder::inherit_jobserver() for external subcommands1, so file descriptors get close due to FD_CLOEXEC flag (on Linux).

Not sure whether this issue is a bug or a feature. GNU Make says2:

If your tool determines that the --jobserver-auth option is available in MAKEFLAGS but that the file descriptors specified are closed, this means that the calling make process did not think that your tool was a recursive make invocation (e.g., the command line was not prefixed with a + character). You should notify your users of this situation.

If I get it correctly, it's up to Cargo to determine whether the process should inherit the jobserver or not. It seems that cargo-fuzz is a reasonable use case and personally I'd love to see it working.

Footnotes

  1. https://github.com/rust-lang/cargo/blob/cd4616448056f56f8b6aeaf0fa632f7f5125638f/src/bin/cargo/main.rs#L176-L184

  2. https://www.gnu.org/software/make/manual/html_node/POSIX-Jobserver.html#POSIX-Jobserver

@weihanglo
Copy link
Member

Hi. We have discussed inheriting jobserver fd on Zulip. There might be some compatibility issues: It's rare but some commands might use fd 3 and 4 without notice of the existence of jobserver. As you said, there is a simple workaround. I'd recommend sticking with that.

@trinity-1686a
Copy link
Author

I understand the compatibility concern.

You should notify your users of this situation.

Could cargo do that, so that at least other peoples stumbling on this bug/feature are made aware of the workaround? There are multiple external subcommands that actually call cargo internally (cargo-fuzz, cargo-tarpaulin...).

Also some such external subcommands do not support being called directly (I'm thinking about cargo-ndk). This is not my use case, but it could me someone else's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-custom-subcommands Area: custom 3rd party subcommand plugins A-jobserver Area: jobserver, concurrency, parallelism C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants