Use exec() to delegate to the real cargo/rustc on Unix

[alexcrichton]

Right now the Command API is presumably used which is cross platform (yay!) but doesn't provide the best experience on Unix. For example if I gdb cargo it doesn't actually gdb the Cargo executable itself but rather the shim. If, however, exec were used to spawn the new process then gdb I believe should naturally "just work".

Plus there's the added benefit of fewer processes sticking around!

[Diggsey]

Sounds reasonable. When I fix #30 you should be able to do multirust proxy gdb cargo too, and it will work regardless.

[nagisa]

Needs more priority. People are getting bitten by this when trying to get backtraces for ICEs.

[Diggsey]

This may be problematic: #972 (comment)

[RalfJung]

@brson

This does solve the problem, but I want to solve these two issues a different way, and the main reason is that rustup wants to be able to perform work after the invoked subcommand is run (e.g. recording telemetry about the results). I am not prepared to give up that capability yet.

What kind of telemetry would that be? Is anything like that actually being done? It seems like currently, there are unsolved issues affecting users caused by using fork-exec, for the benefit of some potential future feature. Or have I entirely missed some rustup functionality? (Wouldn't be the first time.^^)

Maybe another option would be to spawn some "watcher" process, exec in the main process (so signals etc. work properly), and have the "watcher" perform whatever has to be done when the subprocess terminates? Not sure however if that would actually be simpler.

EDIT: I just saw rustup telemetry for the first time. oO Unfortunately, the help message gives 0 indication for what exactly is covered by "telemetry", but I must have entirely missed this.
Sorry.

[geofft]

Maybe another option would be to spawn some "watcher" process, exec in the main process (so signals etc. work properly), and have the "watcher" perform whatever has to be done when the subprocess terminates? Not sure however if that would actually be simpler.

That seems like the right model, and as an example, strace -D does exactly this: "Run tracer process as a detached grandchild, not as parent of the tracee. This reduces the visible effect of strace by keeping the tracee a direct child of the calling process." Compare what happens if you send a Ctrl-C to, say, strace python vs. strace -D python.

We don't want to ptrace the child process (otherwise gdb cargo wouldn't work), so there's not a great cross-platform way to be notified when it exits. But I'm guessing the motivation for telemetry is to track commands known to rustup (rustc and cargo, in particular), and not arbitrary processes run by either rustc run or cargo run.

So that suggests a reasonably straightforward strategy: if rustup is running rustc or cargo, open a libc::pipe(), pass the reader end to the watcher process, and add a --rustup-fd option to rustc and cargo and let the writer end be inherited when you exec those commands. They should learn to close this fd when they exit, or in the case of cargo run, just before they exec the target command. (In the future you could pass info over this fd.) Then the watcher process can just block on reading the pipe. Don't bother setting up a watcher or a pipe for rustup run commands other than rustc or cargo, just exec the command like normal.

Is it reasonable to add this form of rustup awareness to rustc and cargo?

You could also just pass the fd and not tell the child process about it, but leaking file descriptors is vaguely poor form, and if you're using rustup run or cargo run to launch a long-running process, it's nice not to leave the watcher process around.

Also, exempting cargo run from telemetry might be simpler and worth doing as a stopgap - #806 and #845 (marked as dupes of this issue) are specifically about cargo run, and it's unfortunate that cargo itself execs (rust-lang/cargo#2343) but rustup prevents this from doing what you want.

[nagisa]

The writing end would close itself automatically as the cargo or rustc exit without them having any knowledge about the FD anyway.

…

On Jul 12, 2017 9:27 PM, "Geoffrey Thomas" ***@***.***> wrote: Maybe another option would be to spawn some "watcher" process, exec in the main process (so signals etc. work properly), and have the "watcher" perform whatever has to be done when the subprocess terminates? Not sure however if that would actually be simpler. That seems like the right model, and as an example, strace -D does exactly this: "Run tracer process as a detached grandchild, not as parent of the tracee. This reduces the visible effect of strace by keeping the tracee a direct child of the calling process." Compare what happens if you send a Ctrl-C to, say, strace python vs. strace -D python. We don't want to ptrace the child process (otherwise gdb cargo wouldn't work), so there's not a great cross-platform way to be notified when it exits. But I'm guessing the motivation for telemetry is to track commands known to rustup (rustc and cargo, in particular), and not arbitrary processes run by either rustc run or cargo run. So that suggests a reasonably straightforward strategy: if rustup is running rustc or cargo, open a libc::pipe(), pass the reader end to the watcher process, and add a --rustup-fd option to rustc and cargo and let the writer end be inherited when you exec those commands. They should learn to close this fd when they exit, or in the case of cargo run, just before they exec the target command. (In the future you could pass info over this fd.) Then the watcher process can just block on reading the pipe. Don't bother setting up a watcher or a pipe for rustup run commands other than rustc or cargo, just exec the command like normal. Is it reasonable to add this form of rustup awareness to rustc and cargo? You could also just pass the fd and not tell the child process about it, but leaking file descriptors is vaguely poor form, and if you're using rustup run or cargo run to launch a long-running process, it's nice not to leave the watcher process around. Also, exempting cargo run from telemetry might be simpler and worth doing as a stopgap - #806 <#806> and #845 <#845> (marked as dupes of this issue) are specifically about cargo run, and it's unfortunate that cargo itself execs (rust-lang/cargo#2343 <rust-lang/cargo#2343>) but rustup prevents this from doing what you want. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#31 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApc0n5_Wkqjt9YI1IWngUA-kwdNUnigks5sNRATgaJpZM4GZiGs> .