From b3e08ea2e0a3e71d34c1c0fe05ff2133011a80a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Dachary?= Date: Fri, 10 Jun 2022 18:09:56 +0200 Subject: [PATCH] updates to the zombie post To reflect the latest version --- content/blog/2022-06-04-zombies-part-2.md | 31 ++++++++++------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/content/blog/2022-06-04-zombies-part-2.md b/content/blog/2022-06-04-zombies-part-2.md index 8f652a3..b1901d7 100644 --- a/content/blog/2022-06-04-zombies-part-2.md +++ b/content/blog/2022-06-04-zombies-part-2.md @@ -29,26 +29,23 @@ When Gitea timeout on a child, it relies on [os.Process.Kill](https://github.com > If pid is less than -1, then sig is sent to every process in the process group whose ID is -pid. -Which is implemented as follows in the [Friendly Forge Format library](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/f42a29284a5262d3e6f94801089369626c5197f6/util/exec.go#L79): +Which is implemented as follows in the [Friendly Forge Format library](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/a9603c7cc934fccd4382b7f4309b75c852742480/util/exec.go#L130): > `syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)` ### Not using the default Go CommandContext -Since [CommandContext](https://pkg.go.dev/os/exec#CommandContext) does not allow to send a signal to the negative pid of the child process, it has to be implemented by Gitea itself, in a way that is similar to how the [Friendly Forge Format library](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/f42a29284a5262d3e6f94801089369626c5197f6/util/exec.go#L71-87) does it: +Since [CommandContext](https://pkg.go.dev/os/exec#CommandContext) does not allow to send a signal to the negative pid of the child process, it has to be implemented by Gitea itself, in a way that is similar to how the [Friendly Forge Format library](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/a9603c7cc934fccd4382b7f4309b75c852742480/util/exec.go#L75-82) does it: ``` - err := cmd.Start() -... - go func() { - <-ctx.Done() - - if killErr := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); killErr == nil { -... - } - }() - + ctxErr := watchCtx(ctx, cmd.Process.Pid) err = cmd.Wait() + interruptErr := <-ctxErr + // If cmd.Wait returned an error, prefer that. + // Otherwise, report any error from the interrupt goroutine. + if interruptErr != nil && err == nil { + err = interruptErr + } ``` ### Testing the bug is fixed and stays fixed @@ -59,11 +56,11 @@ Long standing bugs that are difficult to reproduce manually such as this one req * the bug fix works * it does not resurface insidiously because of a subtle regression introduce years later -It is easy to implement as can be seen in the [Friendly Forge Format library](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/f42a29284a5262d3e6f94801089369626c5197f6/util/exec_test.go#L44-76). In a nutshell: +It is easy to implement as can be seen in the [Friendly Forge Format library](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/a9603c7cc934fccd4382b7f4309b75c852742480/util/exec_test.go#L44-76). In a nutshell: -* [git clone https://4.4.4.4](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/f42a29284a5262d3e6f94801089369626c5197f6/util/exec_test.go#L53) which will hang because of firewall rules -* [wait for the git-remote-https](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/f42a29284a5262d3e6f94801089369626c5197f6/util/exec_test.go#L60-65) grandchild process to be spawned -* [cancel the context and wait for the goroutine to terminate](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/f42a29284a5262d3e6f94801089369626c5197f6/util/exec_test.go#L67-68) -* [verify the git-remote-https is killed](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/f42a29284a5262d3e6f94801089369626c5197f6/util/exec_test.go#L70-75) +* [git clone https://4.4.4.4](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/a9603c7cc934fccd4382b7f4309b75c852742480/util/exec_test.go#L53) which will hang because of firewall rules +* [wait for the git-remote-https](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/a9603c7cc934fccd4382b7f4309b75c852742480/util/exec_test.go#L60-65) grandchild process to be spawned +* [cancel the context and wait for the goroutine to terminate](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/a9603c7cc934fccd4382b7f4309b75c852742480/util/exec_test.go#L67-68) +* [verify the git-remote-https is killed](https://lab.forgefriends.org/friendlyforgeformat/gofff/-/blob/a9603c7cc934fccd4382b7f4309b75c852742480/util/exec_test.go#L70-75) And with that... no more zombies!