Bug: uiop:terminate-process - doesn't terminate
Hello,
Context
I'm using (uiop:launch-program "bash some_script.sh" :input :stream :output :stream)
to launch scripts and programs asynchronously.
However, very often this does not kill the program, only the parent shell that spawns it.
I'm using SBCL 2.1.8 on Ubuntu 20.04.
Bug
Internally uiop:terminate-process
calls %posix-send-signal
on UNIX, and in turn SBCL's process-kill (for SBCL) - which uses the "kill" system call instead of "killpg", closing the shell and leaving the child processes running.
There is even a comment about it but it's not even correct, as it fails with both SIGTERM and SIGKILL to kill all the spawned processes, killing only the shell:
;; WARNING: For signals other than SIGTERM and SIGKILL this may not
;; do what you expect it to. Sending SIGSTOP to a process spawned
;; via LAUNCH-PROGRAM, e.g., will stop the shell /bin/sh that is used
;; to run the command (via `sh -c command`) but not the actual
;; command.
#+os-unix
(defun %posix-send-signal (process-info signal)
#+allegro (excl.osi:kill (slot-value process-info 'process) signal)
#+clozure (ccl:signal-external-process (slot-value process-info 'process)
signal :error-if-exited nil)
#+(or cmucl scl) (ext:process-kill (slot-value process-info 'process) signal)
#+sbcl (sb-ext:process-kill (slot-value process-info 'process) signal)
#-(or allegro clozure cmucl sbcl scl)
(if-let (pid (process-info-pid process-info))
(symbol-call :uiop :run-program
(format nil "kill -~a ~a" signal pid) :ignore-error-status t)))
Proposal to fix UNIX implementations:
Proposal 1: (don't think it's a good idea)
Instead of uiop:%posix-send-signal
calling kill
on UNIX, call killpg
.
- Pros: At least for SIGTERM everything closes.
- Cons: Brute force. Too aggressive? This suffers from lack of granularity for some use cases and other signals?
Proposal 2:
Like SBCL, add to uiop:%posix-send-signal
an extra optional parameter whom
to %posix-send-signal, which by default is :pid
, but can also be :process-group
- in which case the killpg
system call should be used instead of kill
.
Don't think it's a good idea to start sending signals to the group all the time right?
In such case, terminate-process could have another parameter whom
as well.
- Pros: Remains generic, gains functionality, and it's possible to use
killpg
when calling withterminate-process
, fixing the current bug on UNIX. - Cons: More effort than 1? (but it's very small anyway); Could be too aggressive? and not let the application manage the child processes shutdown?
Proposal 2+ / 3:
Another option would be to kill the shell and the process it spawns with separate kill
commands, but it might not be trivial to figure out which PID that the subprocess has. That would however grant the program the chance to correctly shutdown itself and its child processes.
- Pros: Compatible and synergistic with 2 - when using
kill
. Allows apps that are well built to gracefully shutdown along with their children. - Cons: For badly behaved apps, implementing this and not having proposal 2's
killpg
might not be enough to close all resources.
If you're interested I can gladly submit a patch. I've never contributed to any CL project and would love to do so, thank you :)