dive
dive copied to clipboard
[FEATURE] Add way to cancel job in desktop version
Hit an error with a pipeline running forever
Gathering some knowledge for what is required for this task. https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options
https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal
We don't save the job object, we do save the PID but that is only valid for the life of the process. After the process is complete we need to make sure we aren't killing a different PID.
Checking the code there is a processManager.ts
which seems to have the functionality to close child processes started by the system. Within there is a children
which is a list of all of the child processes. I could add a function here which does a search through existing jobs that haven't ended for the PID and then closes a specific one.
Once that part is setup I just need to add an interface to the jobs panel where if it is running there is a cancel button which will send a signal to the process to exit. Need to check on the subprocess shell issue that occurs with linux shell process commands specifically this quote under the subprocess_kill_signal: "On Linux, child processes of child processes will not be terminated when attempting to kill their parent. This is likely to happen when running a new process in a shell or with the use of the shell option of ChildProcess:"
I don't know how similar Node's runtime is to UNIX proper, but the usual thing to do would be to catch SIGCHLD
(which a child process emits to the parent when it terminates) and use that to update a list of active child processes. You could just check that table when attempting to kill a child process by it's PID. Your approach of checking the list of the processes is probably fine too. And one last idea: it's probably fine to just attempt to kill by PID anyway: for one thing, you're not likely to see a new process with the same PID show up for a very, very long time, and also even if that did happen, you simply wouldn't be able to kill it (unless you're running as root I suppose).
Anyway it's been a while since I worked with these concepts directly, so I may just be blowing smoke. Proceed as you see fit.
Yeah there is infrastructure already for killing by PID and I can use that for the process so that is what I'm using.
To represent the state of a Job we were only using the exitCode
provided by the process, null
for currently running jobs 0
for success and non-0 for failures/errors. Problem is when killing/cancelling a job it just returns signal
that was used instead of updating the exit code so I'll have to switch all reporting of status to be a combination of exitCode/Signal instead of just exitCode.
Node 'exit'
event Quote I'm referencing
The 'exit' event is emitted after the child process ends. If the process exited, code is the final exit code of the process, otherwise null. If the process terminated due to receipt of a signal, signal is the string name of the signal, otherwise null. One of the two will always be non-null.
This is being placed on hold until we can guarantee that there are no abandoned child processes when sending signals to the launching process.
According to this stack overflow answer, this should be possible to do in a Gordian knot kind of way.