runv containerd: race condition between global signal handler for SIGCHILD and cmd.Wait()

[pmorjan]

In a runv containerd setup we noticed that sometimes docker run fails with

docker: Error response from daemon: StartPod fail

The runv error message is

qemu_process.go:142 waitid: no child processes

The problem is that although docker has been notified that the run command failed it actually succeeded. The Qemu process has started successfully and keeps running. But it's not visible for docker.

The root cause seems to be the global signal handler for SIGCHILD https://github.com/hyperhq/runv/blob/master/containerd/containerd.go#L143

In the error case this handler catches the SIGCHILD from the Qemu before cmd.Wait() in qemu_process.go. The signal handler calls osutils.Reap() which then calls Wait4 for the Qemu process. At that point in time the process is gone. Later cmd.Wait() fails with

qemu_process.go:142] waitid: no child processes

That error message is correct. There is no child to wait for any more. https://github.com/hyperhq/runv/blob/master/hypervisor/qemu/qemu_process.go#L142

The cmd.Run() is actually a wrapper: https://golang.org/src/os/exec/exec.go#L266

func (c *Cmd) Run() error {
   if err := c.Start(); err != nil {
      return err
   }
   return c.Wait()
 }

The error happens rarely but you can add some debug code to trigger it on every run by splitting the cmd.Run() into cmd.Start() and cmd.Wait() and a short delay in between.

diff --git a/hypervisor/qemu/qemu_process.go b/hypervisor/qemu/qemu_process.go
index 31f7562..94d41bf 100644
--- a/hypervisor/qemu/qemu_process.go
+++ b/hypervisor/qemu/qemu_process.go
@@ -139,7 +139,9 @@ func launchQemu(qc *QemuContext, ctx *hypervisor.VmContext) {
        cmd.Stdout = stdout
        cmd.Stderr = stderr

-       err := cmd.Run()
+       cmd.Start()
+       time.Sleep(time.Millisecond*100)
+       err := cmd.Wait()

        if stdout.Len() != 0 {
                glog.Info(stdout.String())

[laijs]

thanks for report, could you remove the SIGCHILD from https://github.com/hyperhq/runv/blob/master/containerd/containerd.go#L143

[pmorjan]

sure, I'll create a PR