CRI recovery leaks an unbounded goroutine per sandbox: waitSandboxExit escapes loadContainerTimeout

Summary

podsandbox.(*Controller).RecoverContainer bounds its synchronous body with a 10s loadContainerTimeout, but the goroutine it spawns to wait for sandbox exit is started with a fresh, unbounded context. If the shim behind that sandbox does not answer, the goroutine blocks in a ttrpc call forever. On a node with many sandboxes this leaves hundreds of goroutines parked indefinitely, and we observed the CRI plugin never reaching a ready state after a containerd restart.

Environment

• containerd v2.2.3 (also present on release/2.2 and main)
• CRI plugin, podsandbox sandbox controller (default)
• Runtime: an out-of-tree shim (kata-containers containerd-shim-kata-v2); the shim had an outstanding ttrpc call of its own and did not service new requests
• Kubernetes 1.34

The code

internal/cri/server/podsandbox/recover.go:

func (c *Controller) RecoverContainer(ctx context.Context, cntr containerd.Container) (sandboxstore.Sandbox, error) {
	ctx, cancel := context.WithTimeout(ctx, loadContainerTimeout)   // 10s
	defer cancel()
	...
	if ch != nil {
		go func() {
			if err := c.waitSandboxExit(ctrdutil.NamespacedContext(), podSandbox, ch); err != nil {
				log.G(context.Background()).Warnf("failed to wait pod sandbox exit %v", err)
			}
		}()
	}

loadContainerTimeout correctly bounds getMetadata, cntr.Info, cntr.Task and so on. The spawned goroutine, however, calls waitSandboxExit with ctrdutil.NamespacedContext() rather than the timeout-bounded ctx, so nothing bounds it. defer cancel() fires when RecoverContainer returns, which does not affect the goroutine because it never received that context.

This is identical on v2.2.3, release/2.2 and main, so upgrading does not change the behaviour.

Observed behaviour

After a containerd restart on a node with ~50 sandboxes, the CRI plugin never became available. The daemon kept logging shim cleanup activity, and the kubelet sat in Waiting for containerd startup: rpc error: code = Unavailable until we killed the unresponsive shim processes by hand. Recovery visibly advanced by one step each time a shim died.

SIGUSR1 goroutine dump from the stuck daemon (identifiers elided):

goroutine NNN [select, 160 minutes]:
github.com/containerd/ttrpc.(*Client).Call(...)
...
github.com/containerd/containerd/v2/internal/cri/server/podsandbox.(*Controller).RecoverContainer.func2()
	/internal/cri/server/podsandbox/recover.go:137
created by ...podsandbox.(*Controller).RecoverContainer in goroutine NNNN
	/internal/cri/server/podsandbox/recover.go:136

551 goroutines were parked in [select, 160 minutes] at the time of the dump.

Impact

• One goroutine leaked per sandbox whose shim is unresponsive; they never exit.
• On a node with many sandboxes, the daemon accumulates hundreds of blocked goroutines holding ttrpc client state.
• In our case the CRI plugin never became ready, so the kubelet never started and the node was reported NotReady with NodeStatusUnknown. Recovery required killing the offending shims and restarting the agent.

I want to be precise about what is proven and what is inferred: the unbounded context and the leaked goroutines are directly verifiable from the source and the dump. Whether that leak is the sole reason the CRI plugin never became ready, versus a contributing factor alongside contention on shared ttrpc state, I could not establish from the dump alone.

Why the existing timeouts do not help

[timeouts] defaults in this version are:

'io.containerd.timeout.shim.load'    = '5s'
'io.containerd.timeout.task.state'   = '2s'
'io.containerd.timeout.shim.cleanup' = '5s'
'io.containerd.timeout.shim.shutdown'= '3s'

None of these are consulted on this path. The CRI plugin config also has no relevant knob — only stream_idle_timeout, image_pull_progress_timeout and drain_exec_sync_io_timeout, none of which bound shim reattach.

Suggested fix

Give the wait goroutine a cancellable context tied to the controller's lifetime, so a shim that never answers cannot leak it. Roughly:

if ch != nil {
	go func() {
		wctx, wcancel := context.WithCancel(ctrdutil.NamespacedContext())
		defer wcancel()
		// or a context owned by the controller and cancelled on shutdown
		if err := c.waitSandboxExit(wctx, podSandbox, ch); err != nil {
			log.G(wctx).Warnf("failed to wait pod sandbox exit %v", err)
		}
	}()
}

A plain WithCancel alone does not help unless something cancels it, so the useful shapes are either:

1. tie the context to the controller/daemon lifecycle so shutdown reclaims these goroutines, or
2. bound the initial reattach with a timeout (as the synchronous path already does), and treat a shim that fails to answer within it as unreachable rather than waiting forever.

Option 2 also removes the failure mode where a single unresponsive shim prevents the node from ever becoming ready, which seems worth having regardless of the leak.

Happy to send a PR if you agree on which shape you would prefer.

Reproduction sketch

1. Run a node with a number of sandboxes on a shim that can become unresponsive to ttrpc while its process stays alive.
2. Wedge one shim so it accepts connections but never answers.
3. Restart containerd.
4. Observe the CRI plugin failing to become ready, and RecoverContainer.func2 goroutines parked indefinitely in a SIGUSR1 dump.