Cleaning up resources before `Process.exec`

[straight-shoota]

Process.exec replaces the current process. It happens immediately and affects everything in the process. This means there's no opportunity for any cleanup or dispatch fibers to run before it.

We've experienced a problem in shards when changing from Process.run to Process.exec: crystal-lang/shards#686 (comment) A log message emitted right before the exec call never got written because the async log dispatch fiber never has a chance to process it.

This simplified example demonstrates the problem:

require "log"

Log.info{"foo"}

Process.exec("echo bar")

The info log message doesn't appear in the output.
Adding ::Log.builder.close before the exec call fixes this.

The same problem applies at process exit. For that reason, we have an exit handler to close the log builder

crystal/src/log/main.cr

Line 54 in ef7218b

at_exit { @@builder.close }

But exit handlers only run on process exit, not on exec.

I think we should have some mechanism to enable certain necessary cleanup tasks (such as processing log events) for Process.exec, similar to what we have for exit. It should not be the burden of the code that calls Process.exec to be aware of which things needs to be done before doing that safely.

Maybe we should run exit handlers before exec? In a way it's similar - the current process stops executing. Although there are some slight semantic differences, which might or might not be relevant.

[ysbaddaden]

We barely use at_exit in stdlib. There's closing the Log callback and three callbacks to reset the console on Windows. A last usage is to eventually run the specs. Since exec doesn't work as intended on Windows, that leaves the Log close callback for the at exit handlers.

Eventually, also flushing IO buffers.

[ysbaddaden]

Rust uses the distinct std.process.Command.pre_exec to register specific callbacks to be run before exec. Note that the callbacks are invoked in registration order, while our at exit handlers are run in reverse order.

[straight-shoota]

Command.pre_exec comes from the CommandExt trait on unix and seems to be for the fork + exec use case: It executes after fork in the new process, just before exec.

https://doc.rust-lang.org/stable/std/os/unix/process/trait.CommandExt.html#tymethod.exec

And thus it's a Unix-only API.

[ysbaddaden]

Documentation states so, but in practice the pre_exec closures are executed by do_exec() that is also called by exec(), not only by the fork + exec path of the spawn() function.

It's indeed only implemented for UNIX targets (std::os::unix::process::Command), yet surprisingly both functions are exposed publicly in the general std::process::Command 🤷