Deep Review: 20260324-062450-pr-242

After stopInstanceForcibly kills the process tree, the code waits on <-state.procExited at line 525 with no timeout. This channel closes when cmd.Wait() returns, but cmd.Wait() blocks until all pipe readers close. On Windows, if a child process survives KillTree (e.g., because taskkill /T cannot traverse from a dead parent — documented at process_windows.go:70-76) and holds an inherited pipe, cmd.Wait() never returns and service shutdown hangs.

	for name, state := range states {
		graceful := true
		select {
		case <-state.procExited:
		case <-time.After(gracefulShutdownTimeout):
			graceful = false
		}
		if !graceful {
			// Manager context is cancelled; use a fresh context for cleanup.
			// Inspect before killing so PIDs are still valid (not yet recycled
			// to unrelated processes). stopInstanceForcibly kills the process
			// tree and cleans up PID/socket/tmp files in one pass.
			logger := ctrl.Log.WithName("shutdownAllHostagents")
			cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
			inst, err := store.Inspect(cleanupCtx, name)
			if err == nil && inst != nil {
				stopInstanceForcibly(cleanupCtx, logger, inst)
			}
			cancel()
			<-state.procExited
		}
		state.cancel()
		r.instanceStatesMu.Lock()
		delete(r.instanceStates, name)
		r.instanceStatesMu.Unlock()
	}

The sister function shutdownHostagent (lines 533-536) correctly uses a 10-second bounded wait via waitAfterKill. The pre-existing code fell through to state.cancel() immediately without waiting; this change adds the blocking point without a safety bound.

	// After forced termination, wait briefly for the process to exit.
	// Use a background context (like forceStop above) because the parent
	// reconciler context may be exhausted or cancelled by now.
	waitAfterKill := func() {
		killCtx, killCancel := context.WithTimeout(context.Background(), 10*time.Second)
		defer killCancel()
		r.waitForProcessExit(killCtx, name)
	}

Fix: Add a timeout around the post-kill wait, matching shutdownHostagent:

         stopInstanceForcibly(cleanupCtx, logger, inst)
     }
     cancel()
-    <-state.procExited
+    waitCtx, waitCancel := context.WithTimeout(context.Background(), 10*time.Second)
+    select {
+    case <-state.procExited:
+    case <-waitCtx.Done():
+        logger.Info("Timed out waiting for hostagent to exit after forced kill", "instance", name)
+    }
+    waitCancel()

stopInstanceForcibly() records kill failures internally (allKilled = false at line 624) but does not propagate the result to its caller. handleDeletion() then unconditionally zeroes DriverPID and HostAgentPID at lines 61-68 before calling limainstance.Delete() at line 74. Lima's Delete() calls StopForcibly(inst) internally as a last-resort kill. Zeroing the PIDs disables that retry on the exact path where it matters most — when the first kill failed and the process is still alive.

	if existingInst != nil {
		// Only use PID-based force-stop for Running instances. Broken
		// instances may have stale PID files pointing to recycled processes
		// on Windows (Lima's ReadPIDFile treats any live PID as valid).
		// Not tested: simulating stale PID files requires Windows-specific
		// PID file manipulation that BATS cannot easily reproduce.
		if existingInst.Status == limatype.StatusRunning {
			stopInstanceForcibly(ctx, logger, existingInst)
		} else if existingInst.VMType == limatype.WSL2 {
			// A "stopped" WSL2 distro can retain kernel state that deadlocks
			// wsl.exe --unregister. Terminate it without PID-based killing,
			// since the PIDs may have been recycled on Windows.
			terminateWSL2Distro(ctx, logger, existingInst.Name)
		}
		if runtime.GOOS == "windows" {
			// Clear PIDs so Lima's Delete → StopForcibly does not kill
			// unrelated processes if the PIDs were recycled. Windows recycles
			// PIDs aggressively, and Lima's ReadPIDFile treats any live PID
			// as valid. On Unix, PID recycling is rare (wraps around 32768+),
			// so we let Lima's Delete clean up any surviving driver processes.
			existingInst.DriverPID = 0
			existingInst.HostAgentPID = 0
		}
		logger.Info("Deleting Lima instance", "instance", limaVM.Name)
		// Use a timeout because Lima's WSL2 driver calls wsl.exe --unregister
		// which can hang indefinitely if the WSL subsystem is degraded.
		deleteCtx, deleteCancel := context.WithTimeout(ctx, 60*time.Second)
		err = limainstance.Delete(deleteCtx, existingInst, true)
		deleteCancel()
		if err != nil {
			logger.Error(err, "Failed to delete Lima instance")
			return ctrl.Result{}, err
		}
		logger.Info("Deleted Lima instance", "instance", limaVM.Name)
	}

The intent (prevent killing recycled PIDs) is sound and well-documented. The gap is that when stopInstanceForcibly just ran and failed, the PIDs are still valid — they have not been recycled because the process is still alive.

Meanwhile, stopInstanceForcibly already tracks success internally but discards the result:

func stopInstanceForcibly(ctx context.Context, logger logr.Logger, inst *limatype.Instance) {
	allKilled := true
	for _, pid := range []int{inst.DriverPID, inst.HostAgentPID} {
		if pid > 0 {
			if err := process.KillTree(ctx, pid); err != nil {
				logger.V(1).Info("Failed to kill process tree", "pid", pid, "error", err)
				allKilled = false
			}
		}
	}

Fix: Return a success indicator from stopInstanceForcibly and only zero PIDs when the kill succeeded:

-func stopInstanceForcibly(ctx context.Context, logger logr.Logger, inst *limatype.Instance) {
+func stopInstanceForcibly(ctx context.Context, logger logr.Logger, inst *limatype.Instance) bool {
     allKilled := true
     ...
-}
+    return allKilled
+}

-    if existingInst.Status == limatype.StatusRunning {
-        stopInstanceForcibly(ctx, logger, existingInst)
+    allStopped := true
+    if existingInst.Status == limatype.StatusRunning {
+        allStopped = stopInstanceForcibly(ctx, logger, existingInst)
     }
     if runtime.GOOS == "windows" {
-        existingInst.DriverPID = 0
-        existingInst.HostAgentPID = 0
+        if allStopped {
+            existingInst.DriverPID = 0
+            existingInst.HostAgentPID = 0
+        }
     }