Wait for waitInitExit() to return

gabriel-samfira · gabriel-samfira · commit a6edb2596b40 · 2022-01-06T08:49:14.000+02:00
This change gives waitInitExit() a chance to cleanup resource
when DeleteExec() is called, before returning.

This should fix situations where the shim exits before releasing
container resources.

Signed-off-by: Gabriel Adrian Samfira &lt;gsamfira@cloudbasesolutions.com&gt;
diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs.go b/cmd/containerd-shim-runhcs-v1/task_hcs.go
@@ -590,6 +590,41 @@ func (ht *hcsTask) DeleteExec(ctx context.Context, eid string) (int, uint32, tim
 	case shimExecStateRunning:
 		return 0, 0, time.Time{}, newExecInvalidStateError(ht.id, eid, state, "delete")
 	}
+
+	if eid == "" {
+		// We are killing the init task, so we expect the container to be
+		// stopped after this.
+		//
+		// The task process may have already exited, and the status set to
+		// shimExecStateExited, but resources may still be in the process
+		// of being cleaned up. Wait for ht.closed to be closed. This signals
+		// that waitInitExit() has finished destroying container resources,
+		// and layers were umounted.
+		// If the shim exits before resources are cleaned up, those resources
+		// will remain locked and untracked, which leads to lingering sandboxes
+		// and container resources like base vhdx.
+		select {
+		case <-time.After(30 * time.Second):
+			log.G(ctx).Error("timed out waiting for resource cleanup")
+			return 0, 0, time.Time{}, errors.Wrap(hcs.ErrTimeout, "waiting for container resource cleanup")
+		case <-ht.closed:
+		}
+
+		// The init task has now exited. A ForceExit() has already been sent to
+		// execs. Cleanup execs and continue.
+		ht.execs.Range(func(key, value interface{}) bool {
+			if key == "" {
+				// Iterate next.
+				return true
+			}
+			ht.execs.Delete(key)
+
+			// Iterate all. Returning false stops the iteration. See:
+			// https://pkg.go.dev/sync#Map.Range
+			return true
+		})
+	}
+
 	status := e.Status()
 	if eid != "" {
 		ht.execs.Delete(eid)
diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs_test.go b/cmd/containerd-shim-runhcs-v1/task_hcs_test.go
@@ -161,6 +161,8 @@ func Test_hcsTask_DeleteExec_InitExecID_CreatedState_Success(t *testing.T) {
 	// remove the 2nd exec so we just check without it.
 	lt.execs.Delete(second.id)
 
+	// Simulate waitInitExit() closing the host
+	close(lt.closed)
 	// try to delete the init exec
 	pid, status, at, err := lt.DeleteExec(context.TODO(), "")
 
@@ -178,6 +180,8 @@ func Test_hcsTask_DeleteExec_InitExecID_RunningState_Error(t *testing.T) {
 	// Start the init exec
 	_ = init.Start(context.TODO())
 
+	// Simulate waitInitExit() closing the host
+	close(lt.closed)
 	// try to delete the init exec
 	pid, status, at, err := lt.DeleteExec(context.TODO(), "")
 
@@ -192,6 +196,8 @@ func Test_hcsTask_DeleteExec_InitExecID_ExitedState_Success(t *testing.T) {
 
 	_ = init.Kill(context.TODO(), 0xf)
 
+	// Simulate waitInitExit() closing the host
+	close(lt.closed)
 	// try to delete the init exec
 	pid, status, at, err := lt.DeleteExec(context.TODO(), "")
 
@@ -207,6 +213,8 @@ func Test_hcsTask_DeleteExec_InitExecID_2ndExec_CreatedState_Error(t *testing.T)
 	// start the init exec (required to have 2nd exec)
 	_ = init.Start(context.TODO())
 
+	// Simulate waitInitExit() closing the host
+	close(lt.closed)
 	// try to delete the init exec
 	pid, status, at, err := lt.DeleteExec(context.TODO(), "")
 
@@ -226,6 +234,8 @@ func Test_hcsTask_DeleteExec_InitExecID_2ndExec_RunningState_Error(t *testing.T)
 	// put the 2nd exec into the running state
 	_ = second.Start(context.TODO())
 
+	// Simulate waitInitExit() closing the host
+	close(lt.closed)
 	// try to delete the init exec
 	pid, status, at, err := lt.DeleteExec(context.TODO(), "")
 
@@ -244,6 +254,8 @@ func Test_hcsTask_DeleteExec_InitExecID_2ndExec_ExitedState_Success(t *testing.T
 	// put the 2nd exec into the exited state
 	_ = second.Kill(context.TODO(), 0xf)
 
+	// Simulate waitInitExit() closing the host
+	close(lt.closed)
 	// try to delete the init exec
 	pid, status, at, err := lt.DeleteExec(context.TODO(), "")
 
