Repository navigation
fix: kill the whole engine process tree on timeout and run tini as PID 1 - #67
Open
PatrickSt1991 wants to merge 1 commit into
Open
PatrickSt1991 wants to merge 1 commit into
PatrickSt1991 wants to merge 1 commit into
Conversation
execPromise used child_process.exec with the built-in timeout, which only signals the shell. ffsubsync/alass and the ffmpeg processes they spawn kept running to completion after a timeout, and because node is PID 1 in the container they were re-parented to node and left as <defunct> zombies once they exited (13 of them after two weeks on my instance). - spawn the command detached so it leads its own process group, and on timeout kill the group with SIGTERM, then SIGKILL after 5s - keep stdout/stderr on the rejection like before, capped at 10 MB - add tini as PID 1 so any process that still ends up orphaned is reaped - add a regression test that checks a backgrounded grandchild is dead after a timeout
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After running subsyncarr for a couple of weeks I found 13
<defunct>processes on the host, all children of the subsyncarr node process: pairs offfsubsync+ffmpeg, plus a fewalass.Cause:
execPromiseinsrc/helpers.tsuseschild_process.execwith thetimeoutoption. When the timeout fires, node only sends SIGTERM to the shell it spawned.ffsubsync/alassand theffmpegthey start are not signalled and keep running until they finish on their own. So the timeout frees the queue but does not actually stop the work, and the CPU stays busy.entrypoint.shdoesexec gosu node "$@", so node is PID 1. When the orphaned engine processes finally exit they are re-parented to node, which never callswait()on processes it did not spawn, and they stay as zombies until the container is restarted.Fix
execPromisenow usesspawnwithshell: trueanddetached: true, so the command leads its own process group. On timeout the whole group is killed withkill(-pid, SIGTERM), followed by SIGKILL after 5 seconds.stdout/stderr/code/signal/killedare still attached to the rejection, so the engine wrappers keep working unchanged. The error message contains "timed out", which the existingisTimeoutchecks in the engine files match.tiniand uses it as the entrypoint in front ofentrypoint.sh. It only reaps orphans and forwards signals;gosustill drops to thenodeuser as before.src/__tests__/helpers.test.ts: runssleep 30 & echo $!; waitwith a 300 ms timeout and asserts the backgroundedsleepis gone afterwards. With the oldexecbased code that grandchild survives.Notes
detachedis not used and only the direct child is killed, same as before.