Skip to content

orchestrator: node has no independent sandbox timeout enforcement — VMs leak indefinitely if API evictor stops #3193

Description

@AdaAibaby

Summary

The orchestrator node has no independent mechanism to kill sandboxes when their TTL expires. A WaitForExit() function exists that would implement this, but it is never called anywhere in the codebase. As a result, the API-layer evictor is the sole mechanism that enforces sandbox timeouts — making it a single point of failure.

Root Cause

1. WaitForExit is dead code

packages/orchestrator/pkg/sandbox/sandbox.go:1719 defines:

func (s *Sandbox) WaitForExit(ctx context.Context) error {
    timeout := time.Until(s.GetEndAt())
    select {
    case <-time.After(timeout):
        return errors.New("waiting for exit took too long")
    case <-ctx.Done():
        return nil
    case <-s.exit.Done():
        ...
    }
}

A grep over the entire repository confirms zero call sites — this function is never invoked.

2. The lifecycle goroutine only waits for Firecracker to exit naturally

setupSandboxLifecycle (packages/orchestrator/pkg/server/sandboxes.go:1039) runs:

go func() {
    ctx, _ = tracer.Start(context.WithoutCancel(ctx), ...)

    waitErr := sbx.Wait(ctx)  // blocks on exit.WaitWithContext — only unblocks when FC exits
    ...
    sbx.Close(ctx)
}()

sbx.Wait() blocks until the Firecracker process exits or Stop() is called from outside. There is no timeout race. The node's in-memory endAt field is updated via gRPC Update, but no goroutine on the node ever reads it to trigger a kill.

3. The only timeout-kill path is the API evictor

packages/api/internal/orchestrator/evictor/evict.go polls Redis every 50 ms:

ticker := time.NewTicker(50 * time.Millisecond)
// ...
sbxs, _ := e.store.ExpiredItems(ctx)  // ZRANGEBYSCORE -inf now
for _, item := range sbxs {
    go e.evictSandbox(ctx, item)  // gRPC Remove → node.Stop()
}

This is the only code path that kills an expired sandbox. The node itself does nothing.

Failure Scenario

  1. Sandbox created with endAt = now + 5m
  2. API service pod restarts (rolling deploy, OOM, crash)
  3. Evictor goroutine stops — expired sandboxes are no longer discovered
  4. sbx.Wait() in the lifecycle goroutine blocks forever (no timeout)
  5. Firecracker VM continues running on the node indefinitely
  6. Every node accumulates zombie VMs; CPU/memory leak unbounded until node reboot

This also means any transient API outage (even seconds) can cause VMs to outlive their TTL — the evictor catches up only after the API recovers and the next 50 ms tick fires.

Evidence

$ grep -rn "WaitForExit" packages/ --include="*.go"
packages/orchestrator/pkg/sandbox/sandbox.go:1719:func (s *Sandbox) WaitForExit(ctx context.Context) error {

One result — the definition. No callers.

Proposed Fix

Wire WaitForExit into setupSandboxLifecycle so the node enforces timeouts independently:

func (s *Server) setupSandboxLifecycle(ctx context.Context, sbx *sandbox.Sandbox) {
    go func() {
        ctx, childSpan := tracer.Start(context.WithoutCancel(ctx), "stop sandbox-lifecycle", trace.WithNewRoot())
        defer childSpan.End()

        waitErr := sbx.WaitForExit(ctx)
        if waitErr != nil {
            sbxlogger.I(sbx).Error(ctx, "sandbox lifecycle ended", zap.Error(waitErr))
            // If we timed out, the sandbox is still running — stop it.
            if stopErr := sbx.Stop(ctx); stopErr != nil {
                sbxlogger.I(sbx).Error(ctx, "failed to stop sandbox after timeout", zap.Error(stopErr))
            }
        }

        if cleanupErr := sbx.Close(ctx); cleanupErr != nil {
            sbxlogger.I(sbx).Error(ctx, "failed to cleanup sandbox", zap.Error(cleanupErr))
        }

        if closeErr := s.proxy.RemoveFromPool(sbx.LifecycleID); closeErr != nil {
            sbxlogger.I(sbx).Warn(ctx, "errors when manually closing connections to sandbox", zap.Error(closeErr))
        }

        sbxlogger.E(sbx).Info(ctx, "Sandbox stopped")
    }()
}

Note: WaitForExit computes the timeout once (time.Until(GetEndAt())). A follow-up improvement would be to make this timer resettable so that KeepAlive extensions (via gRPC UpdateSetEndAt) are respected without waiting for the next API-driven eviction. For now, the node-side kill serves as a safety net against indefinite leaks.

Impact

  • Severity: High — resource leak, potential node exhaustion
  • Trigger: Any API service restart or sustained outage
  • Components: packages/orchestrator (node), packages/api (evictor)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions