Hostea
/
website
3
2
Fork 2
Code Issues 5 Pull Requests Projects Releases Wiki Activity

updates to the zombie post
ci/woodpecker/push/woodpecker Pipeline was successful Details 

To reflect the latest version
pull/36/head
Loïc Dachary 2022-06-10 18:09:56 +02:00
parent d8676b8891
commit b3e08ea2e0
Signed by: dachary
GPG Key ID: 992D23B392F9E4F2
1 changed files with 14 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
content/blog/2022-06-04-zombies-part-2.md
View File

@ -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!