Skip to content

Fix #110: don't show error message on shutdown

Baptiste Careil requested to merge bcareil/reaction:dev/shutdown into main

The stream_manager is now the sole owner of the child process handle and is responsible for cleaning it.

The stream_manager expects a shutdown command from the broadcast channel it receives as parameter and watches it concurrently with the child process I/O.

When the shutdown command is received, the child process is killed and the remaining I/O is processed. Then the child process is reaped. Or it is killed and reaped after EOF is encountered, would the child process exit before (or at the same time as) the shutdown command is issued.

I noticed you removed some features from the futures dependency as I just added a usage of the select! macro that, as far as I can tell, requires de default feature set to be available. If you don't want to add them back, can look to make it work with tokio's select instead, but the two behave differently and I'd have to do some research first. So, in the meantime, I am opening the MR as to have a first look.

Though there is something bugging me with that change (and what it replaces): we are killing the process using SIGKILL. It means we're not giving a chance to the child process to forward the last bits of data that might be buffered (journalctl buffers its output for example). Generally speaking, commands that buffer their output don't play too well with reaction (because it might delay the corresponding action indefinitely), but we might still want to consider it?

If yes, the code will be a bit more involved. We'll need to use the kill command (either from the nix crate or from libc) ourselves and we'll have to process in three stages:

  1. wait for either the shutdown signal or EOF

Now either kill nicely (SIGINT or SIGTERM) or reap the process (if already dead, likely if we encountered EOF)

  1. wait for either the process termination or a timer that defines the "grace time" the child process has to close on it's own

Now kill the process for good if not already dead

  1. wait for the process termination (we may want to add a timer here as well but it's usually unnecessary as it would only timeout if the child process is stuck in the kernel. For example: it was accessing an NFS mount that is now disconnected. The only solution then is to orphan the child process which will most likely become systemd's burden until a reboot)

During all 3 wait loops, we also need to process I/O from the child process as reaping it (with, e.g., tokio's Child::wait) might block until all the process output is consumed.

Oh, and outright killing a process has a small chance of producing truncated lines on our side as well, so we may want to look into that too.

I added a test jsonnet file to the tests folder for lack of a better location. I considered the config folder, but the files in there look more like examples. I can either remove it or move it elsewhere. I tested this change with this command: cargo run -- start -c tests/test-shutdown.jsonnet -s ./sock & sleep 10 && kill $! && wait

Merge request reports