Plugin process children do not get killed

[j-lanson]

Plugin processes are std::process:Child objects, the only way to delete them in an OS-agnostic way is the Child::kill() function. Per the docs, this is equivalent to a SIGKILL in Unix, which cannot be caught, and therefore handled. This means that when the immediate child process (the plugin entrypoint) dies, it does not get any opportunity to clean up any of its own children, who therefore persist after the parent has been killed.

For example, using a uv run entrypoint to start a python plugin causes a grandchild python3 process to start. When hipcheck core kills the uv run process, the python3 grandchild sticks around because it received no signal.

It would be preferable to be able to send a less "harsh" signal to the child process so it can respond and exit gracefully, or semantically to say that when the gRPC connection closes, the plugin should shut down. In the meantime, I will probably update the python SDK to track its own parent PID and exit when the parent has been killed.

[j-lanson]

@alilleybrinker any thoughts on this?

[alilleybrinker]

Ah, yeah this seems like a problem. We don't want Hipcheck plugins to be living on as zombies after Hipcheck itself closes. I do think giving plugins some grace window to shut themselves down would be good, and then probably falling back to hard-killing them when the timeout is hit. We probably also do want to update our SDKs to handle graceful shutdown, and to document the need to handle shutdown correctly if you're writing a plugin without an SDK.

[j-lanson]

Ah, yeah this seems like a problem. We don't want Hipcheck plugins to be living on as zombies after Hipcheck itself closes. I do think giving plugins some grace window to shut themselves down would be good, and then probably falling back to hard-killing them when the timeout is hit. We probably also do want to update our SDKs to handle graceful shutdown, and to document the need to handle shutdown correctly if you're writing a plugin without an SDK.

The problem is that we technically don't know how many grandchild processes were created by the plugin process in order to kill them as well. We could maybe use a library to get that information and send the same kill signal to all descendants.

[alilleybrinker]

Would setting the process group be sufficient? https://doc.rust-lang.org/std/os/unix/process/trait.CommandExt.html#tymethod.process_group

[j-lanson]

At the very least, we should clearly discuss this limitation in the website docs about making plugins so authors can understand potential issues when spawning children