From 0ac3cd1be170d180b2baed755e8f0da547ceb267 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kenfe-Micka=C3=ABl=20Laventure?= Date: Fri, 15 Jul 2016 11:49:43 -0700 Subject: [PATCH] Fix shim deadlock when joining an existing pid namespace (#290) Signed-off-by: Kenfe-Mickael Laventure --- containerd-shim/main.go | 1 + runtime/runtime_test.go | 12 ++++++++---- supervisor/delete.go | 4 ++++ supervisor/exit.go | 8 +++++--- supervisor/supervisor.go | 1 - supervisor/worker.go | 3 +++ 6 files changed, 21 insertions(+), 8 deletions(-) diff --git a/containerd-shim/main.go b/containerd-shim/main.go index ddfee9a..a865931 100644 --- a/containerd-shim/main.go +++ b/containerd-shim/main.go @@ -115,6 +115,7 @@ func start(log *os.File) error { // runtime has exited so the shim can also exit if exitShim { // Let containerd take care of calling the runtime delete + f.Close() p.Wait() return nil } diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 7b737a5..3581493 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -1,6 +1,7 @@ package runtime import ( + "flag" "fmt" "io" "os" @@ -9,14 +10,13 @@ import ( "syscall" "testing" "time" - "flag" utils "github.com/docker/containerd/testutils" ) var ( - devNull = "/dev/null" - stdin io.WriteCloser + devNull = "/dev/null" + stdin io.WriteCloser runtimeTool = flag.String("runtime", "runc", "Runtime to use for this test") ) @@ -163,13 +163,17 @@ func BenchmarkBusyboxSh(b *testing.B) { } func benchmarkStartContainer(b *testing.B, c Container, s Stdio, bundleName string) { - if _, err := c.Start("", s); err != nil { + p, err := c.Start("", s) + if err != nil { b.Fatalf("Error starting container %v", err) } kill := exec.Command(c.Runtime(), "kill", bundleName, "KILL") kill.Run() + p.Wait() + c.Delete() + // wait for kill to finish. selected wait time is arbitrary time.Sleep(500 * time.Millisecond) diff --git a/supervisor/delete.go b/supervisor/delete.go index 931ad8c..95720d5 100644 --- a/supervisor/delete.go +++ b/supervisor/delete.go @@ -14,6 +14,7 @@ type DeleteTask struct { Status int PID string NoEvent bool + Process runtime.Process } func (s *Supervisor) delete(t *DeleteTask) error { @@ -22,6 +23,9 @@ func (s *Supervisor) delete(t *DeleteTask) error { if err := s.deleteContainer(i.container); err != nil { logrus.WithField("error", err).Error("containerd: deleting container") } + if t.Process != nil { + t.Process.Wait() + } if !t.NoEvent { s.notifySubscribers(Event{ Type: StateExit, diff --git a/supervisor/exit.go b/supervisor/exit.go index f96d925..442327d 100644 --- a/supervisor/exit.go +++ b/supervisor/exit.go @@ -46,9 +46,10 @@ func (s *Supervisor) exit(t *ExitTask) error { } container := proc.Container() ne := &DeleteTask{ - ID: container.ID(), - Status: status, - PID: proc.ID(), + ID: container.ID(), + Status: status, + PID: proc.ID(), + Process: proc, } s.delete(ne) @@ -72,6 +73,7 @@ func (s *Supervisor) execExit(t *ExecExitTask) error { if err := container.RemoveProcess(t.PID); err != nil { logrus.WithField("error", err).Error("containerd: find container for pid") } + t.Process.Wait() s.notifySubscribers(Event{ Timestamp: time.Now(), ID: t.ID, diff --git a/supervisor/supervisor.go b/supervisor/supervisor.go index d900bb0..9055ac0 100644 --- a/supervisor/supervisor.go +++ b/supervisor/supervisor.go @@ -282,7 +282,6 @@ func (s *Supervisor) SendTask(evt Task) { func (s *Supervisor) exitHandler() { for p := range s.monitor.Exits() { - p.Wait() e := &ExitTask{ Process: p, } diff --git a/supervisor/worker.go b/supervisor/worker.go index e226c21..ac7735d 100644 --- a/supervisor/worker.go +++ b/supervisor/worker.go @@ -51,6 +51,7 @@ func (w *worker) Start() { evt := &DeleteTask{ ID: t.Container.ID(), NoEvent: true, + Process: process, } w.s.SendTask(evt) continue @@ -66,6 +67,7 @@ func (w *worker) Start() { evt := &DeleteTask{ ID: t.Container.ID(), NoEvent: true, + Process: process, } w.s.SendTask(evt) continue @@ -76,6 +78,7 @@ func (w *worker) Start() { evt := &DeleteTask{ ID: t.Container.ID(), NoEvent: true, + Process: process, } w.s.SendTask(evt) continue