Add killsignal and killtimeout to exec/rtsp

[tsightler]

@AlexxIT If I understand the fix for #894, it seems that exec processes are now sent KILL signal vs more traditional TERM. SIGKILL forcefully terminates the process while SIGTERM gives the process the opportunity to gracefully cleanup and exit.

This has caused go2rtc to no longer be viable for my use case as, in ring-mqtt, I use exec to execute a simple bash script that runs various processes required to setup and start the stream and forward it via RTSP to go2rtc. When the consumer ends, I would normally get a TERM signal, and the script traps that signal and runs a cleanup function which tells functions to stop, kills any child processes, and exits.

However, it's not possible to trap KILL, so the process is just gone, while the child processes remain and nothing upstream knows about it. While I could potentially work around some of this in my own code, I feel like this fix is wrong and too heavy handed, processes should not be sent KILL unless they fail to respond to a normal TERM request.

Here's a short log snippet showing the process being killed immediately:

12:51:43.135 DBG [streams] stop producer url=exec:/app/ring-mqtt/scripts/start-stream.sh 7c010a706646 live ring/a9ce28cb-fafb-421c-4508-29d54f48cbd3/camera/7c010a706646 {output}
12:51:43.135 DBG [exec] kill rtsp

[AlexxIT]

When the consumer ends, I would normally get a TERM signal

I don't understand, how you get it earlier. In previous versions go2rtc stops receiving data from exec process. So, in usual situation, ffmpeg will fail end exit. But sometimes ffmpeg keep working. This was a problem.

[tsightler]

Perhaps I'm incorrect about how the stop process worked previously, what was the previous method that was used to stop the exec process? Was it dependent on the RTSP processes disconnecting the producer and the ffmpeg process stopping on it's own in that case? If so, that's probably why it worked, because that would still trigger the process for my script to clean itself up and exit.

Regardless, I think it is poor practice to perform a non-graceful process kill as it is common for processes to have cleanup steps before existing, plus it will not clean up any child process, and if you can execute anything, you should not assume that the command being executed has no child processes. That is the core problem in my case, the script which started by go2rtc is killled as it should be, but the child processes stay around because kill can't be trapped by the parent process so it can't perform cleanup. I could implement checks for this in the child process, but that is more of an ugly hack than the correct behavior.

That being said, it seems that it is a common practice to use kill in Golang, perhaps because it looks to be the only method implemented cross platform (Windows doesn't really have signals). Ideally I think it is far more correct to shutdown the process gracefully and only kill it if it fails to do so (that is standard Unix for as long as I can remember). I looked at some other projects, like mediamtx, which as you probably know works similar to go2rtc in that it can spawn a command when a streaming client connects, and, for unix/linux, it uses syscall.Kill(-cmd.Process.Pid, syscall.SIGINT) while for Windows it uses appears process groups and closes the group.

[tsightler]

Sorry, I misread your post, you already answered my question about how the old process worked. So the reason it worked previously is that, with the old behavior, the ffmpeg process, which in my case is started by a completely different process, still exited, and this would cause the other process to send a signal to my script that the process had gone inactive, and the script would cleanup and exit.

Now, go2rtc kills the process and there's no chance for it to clean up it's child processes, plus the ffmpeg process stays active.

There's a bunch of ugly hacks I can do, such as spawning the child process with a wrapper that checks if the parent is killed, and does the cleanup, or implement some type of heartbeat process with the secondary process (a Nodejs server that maintains the API connection to the cloud based cameras) but anything other than SIGKILL is all that is really needed for my case. I honestly, find it amazing that Golang defaults to kill.

[AlexxIT]

v1.9.0 has custom kill signal support https://github.com/AlexxIT/go2rtc?tab=readme-ov-file#source-exec

[tsightler]

Ah, I had missed that. I'll give it a shot, thanks!

[tsightler]

If I'm reading the documentation and the code correctly, kill signal support is only available when used with pipe, while in my case I'm not using pipe.

[AlexxIT]

You right. I needs to add this func to exec/rtsp.

[AlexxIT]

Added support to latest master version

[tsightler]

Thank you very much @AlexxIT!

[AlexxIT]

https://github.com/AlexxIT/go2rtc/releases/tag/v1.9.4