Deep Review: 20260408-145125-pr-280

Every other shutdown path calls stopHostSwitch alongside stopWatcher: stopInstance at line 502, handleDeletion at line 40, handleWatchedState crash recovery at line 213, and shutdownAllHostagents at line 547. The handleRestartNeeded(phaseRunning) branch is the only path that stops the watcher without stopping the host-switch. On the next reconcile, startHostSwitch overwrites the map entry (line 58) without cancelling the old goroutine, leaking a vsock listener, a virtual network, and the errgroup goroutines.

While handleRestartNeeded predates this PR (confirmed via git blame), the lifecycle pattern it should follow was introduced by this PR.

func (r *LimaVMReconciler) handleRestartNeeded(ctx context.Context, limaVM *v1alpha1.LimaVM, phase instancePhase) (ctrl.Result, error) {
	logger := log.FromContext(ctx)

	switch phase {
	case phaseRunning:
		logger.Info("Restart needed: stopping running instance")
		r.shutdownHostagent(ctx, limaVM.Name, nil)
		r.stopWatcher(limaVM.Name)

		// Clear restartNeeded and set Stopped condition in one write.
		// This is inlined (rather than calling stopInstance) so both changes
		// land in a single patch — stopInstance's updateCondition would take
		// its own DeepCopy and miss the RestartNeeded change.
		patch := client.MergeFrom(limaVM.DeepCopy())
		limaVM.Status.RestartNeeded = false
		r.setCondition(limaVM, ConditionRunning, metav1.ConditionFalse, ReasonStopped, "Stopped for restart")
		return ctrl.Result{}, r.Status().Patch(ctx, limaVM, patch)

Fix:

 	case phaseRunning:
 		logger.Info("Restart needed: stopping running instance")
 		r.shutdownHostagent(ctx, limaVM.Name, nil)
 		r.stopWatcher(limaVM.Name)
+		r.stopHostSwitch(limaVM.Name)

startHostSwitch unconditionally overwrites hostSwitchStates[name] at line 58. If the map already has an entry for this instance (from a previous call), the old goroutine's cancel function is discarded, and the goroutine runs until the manager shuts down.

This manifests through two paths. First, startInstance calls startHostSwitch at line 446, but neither the haCmd.Start() failure at line 454 nor the waitForPIDFile failure at line 463 calls stopHostSwitch. On retry, startHostSwitch orphans the previous goroutine. Second, any code path that calls startHostSwitch twice for the same instance (e.g., I1's restart path) leaks the first goroutine.

// startHostSwitch launches the host-switch goroutine for a WSL2 instance.
// It must be called before the hostagent starts, because the guest's
// network-setup.service performs a vsock handshake during early boot.
func (r *LimaVMReconciler) startHostSwitch(ctx context.Context, name string, inst *limatype.Instance) {
	if inst.VMType != limatype.WSL2 {
		return
	}

	hsCtx, hsCancel := context.WithCancel(ctx)
	done := make(chan struct{})

	r.hostSwitchMu.Lock()
	r.hostSwitchStates[name] = &hostSwitchState{
		cancel: hsCancel,
		done:   done,
	}
	r.hostSwitchMu.Unlock()

	logger := logr.FromContextOrDiscard(ctx).WithValues("instance", name, "component", "host-switch")
	go r.runHostSwitch(hsCtx, logger, done)
}

	// Start the host-switch virtual network for WSL2 instances. This must
	// happen before the hostagent starts, because the guest's
	// network-setup.service performs a vsock handshake during early boot.
	r.startHostSwitch(ctx, limaVM.Name, inst)

	// Start hostagent in background.
	haCmd := exec.CommandContext(ctx, rddPath, args...)
	process.SetGroup(haCmd)
	haCmd.Stdout = haStdoutW
	haCmd.Stderr = haStderrW

	if err := haCmd.Start(); err != nil {
		logger.Error(err, "Failed to start hostagent")
		if updateErr := r.updateCondition(ctx, limaVM, ConditionRunning, metav1.ConditionFalse, ReasonStartFailed, err.Error()); updateErr != nil {
			logger.Error(updateErr, "Failed to update status after hostagent start failure")
		}
		return ctrl.Result{}, err
	}

	// Wait for PID file to be created (indicates hostagent has started)
	if err := r.waitForPIDFile(haPIDPath, haStderrPath); err != nil {
		logger.Error(err, "Hostagent did not start")
		if updateErr := r.updateCondition(ctx, limaVM, ConditionRunning, metav1.ConditionFalse, ReasonStartFailed, err.Error()); updateErr != nil {
			logger.Error(updateErr, "Failed to update status after hostagent startup failure")
		}
		return ctrl.Result{}, err
	}

Fix (two parts): Make startHostSwitch idempotent — stop any existing goroutine before starting a new one:

 func (r *LimaVMReconciler) startHostSwitch(ctx context.Context, name string, inst *limatype.Instance) {
 	if inst.VMType != limatype.WSL2 {
 		return
 	}

+	r.stopHostSwitch(name)
+
 	hsCtx, hsCancel := context.WithCancel(ctx)

Add cleanup on startInstance error paths after line 446:

 	if err := haCmd.Start(); err != nil {
 		logger.Error(err, "Failed to start hostagent")
+		r.stopHostSwitch(limaVM.Name)
 		...
 		return ctrl.Result{}, err
 	}

 	if err := r.waitForPIDFile(haPIDPath, haStderrPath); err != nil {
 		logger.Error(err, "Hostagent did not start")
+		r.stopHostSwitch(limaVM.Name)
 		...
 		return ctrl.Result{}, err
 	}

attemptHandshake spawns a new detached goroutine every second (line 299). Each goroutine blocks in hvsock.Dial, which accepts no context and has no built-in timeout. On a host with several unresponsive Hyper-V VMs, goroutines accumulate at one per GUID per second.

Worse, these detached goroutines outlive the WaitGroup. When getVMGUID receives a match from found and returns, defer cancel() fires, all attemptHandshake functions return, wg.Wait() completes, and close(found) executes. A detached goroutine that completed its dial before cancellation but reaches the found <- vmGUID send after close(found) triggers a panic. The select { ... default: } does not protect against sends on a closed channel.

Additionally, readSignature (line 331) and signalListenerReady (line 340) perform I/O without deadlines. A peer that connects but sends incomplete data blocks the goroutine indefinitely.

func attemptHandshake(ctx context.Context, logger logr.Logger, vmGUID hvsock.GUID, found chan<- hvsock.GUID) {
	ticker := time.NewTicker(time.Second)
	defer ticker.Stop()

	for {
		select {
		case <-ctx.Done():
			return
		case <-ticker.C:
			// Dial in a goroutine to avoid blocking on hvsock.Dial timeout.
			go func() {
				conn, err := getVsockConnection(vmGUID, vsockHandshakePort)
				if ctx.Err() != nil {
					if conn != nil {
						conn.Close()
					}
					return
				}
				if err != nil {
					logger.V(1).Info("Handshake dial failed", "vmGUID", vmGUID.String(), "error", err)
					return
				}

				sig, err := readSignature(conn)
				conn.Close()
				if err != nil {
					logger.V(1).Info("Handshake read failed", "vmGUID", vmGUID.String(), "error", err)
					return
				}
				if sig == signaturePhrase {
					logger.V(1).Info("Signature matched", "vmGUID", vmGUID.String())
					select {
					case found <- vmGUID:
					default:
					}
				}
			}()
		}
	}
}

// readSignature reads the signature phrase from the peer.
func readSignature(conn net.Conn) (string, error) {
	buf := make([]byte, len(signaturePhrase))
	if _, err := io.ReadFull(conn, buf); err != nil {
		return "", err
	}
	return string(buf), nil
}

// signalListenerReady tells the guest that the data listener is ready.
func signalListenerReady(vmGUID hvsock.GUID) error {
	conn, err := getVsockConnection(vmGUID, vsockHandshakePort)
	if err != nil {
		return err
	}
	defer conn.Close()
	_, err = conn.Write([]byte(readySignal))
	return err
}

Fix: Make the handshake sequential and add I/O deadlines. This eliminates the goroutine leak, the panic risk, and the missing-deadline problem in one change:

func attemptHandshake(ctx context.Context, logger logr.Logger, vmGUID hvsock.GUID, found chan<- hvsock.GUID) {
	for {
		if ctx.Err() != nil {
			return
		}
		conn, err := getVsockConnection(vmGUID, vsockHandshakePort)
		if err != nil {
			select {
			case <-ctx.Done():
				return
			case <-time.After(time.Second):
			}
			continue
		}
		conn.SetReadDeadline(time.Now().Add(5 * time.Second))
		sig, err := readSignature(conn)
		conn.Close()
		if err != nil {
			logger.V(1).Info("Handshake read failed", "vmGUID", vmGUID.String(), "error", err)
			continue
		}
		if sig == signaturePhrase {
			logger.V(1).Info("Signature matched", "vmGUID", vmGUID.String())
			found <- vmGUID
			return
		}
		// Wrong VM — stop probing this GUID.
		return
	}
}

Also add a write deadline in signalListenerReady:

 func signalListenerReady(vmGUID hvsock.GUID) error {
 	conn, err := getVsockConnection(vmGUID, vsockHandshakePort)
 	if err != nil {
 		return err
 	}
 	defer conn.Close()
+	conn.SetWriteDeadline(time.Now().Add(5 * time.Second))
 	_, err = conn.Write([]byte(readySignal))
 	return err
 }

After vsockHandshake returns a listener at line 96, two error paths return without closing it: virtualnetwork.New failure (line 104) and vn.Listen failure (line 135). The listener holds an AF_VSOCK socket bound to a VM GUID + service ID. Each leaked listener blocks that service ID until the socket is garbage collected.

func (r *LimaVMReconciler) runHostSwitch(ctx context.Context, logger logr.Logger, done chan struct{}) {
	defer close(done)

	subnet, err := validateSubnet(defaultSubnet)
	if err != nil {
		logger.Error(err, "Invalid subnet configuration")
		return
	}

	ln, err := vsockHandshake(ctx, logger)
	if err != nil {
		logger.Error(err, "Vsock handshake failed")
		return
	}

	cfg := newVirtualNetworkConfig(*subnet)
	vn, err := virtualnetwork.New(&cfg)
	if err != nil {
		logger.Error(err, "Failed to create virtual network")
		return
	}

	g, ctx := errgroup.WithContext(ctx)

	// Accept vsock connections and feed them into the virtual network.
	g.Go(func() error {
		for {
			conn, err := ln.Accept()
			if err != nil {
				// Listener closed (context cancelled).
				return nil //nolint:nilerr // Expected on shutdown.
			}
			if err := vn.AcceptStdio(ctx, conn); err != nil {
				logger.Error(err, "Failed to accept connection into virtual network")
			} else {
				logger.Info("Accepted vsock connection")
			}
		}
	})

	// Close the listener when the context is cancelled.
	g.Go(func() error {
		<-ctx.Done()
		return ln.Close()
	})

	// Start the port-forwarding API server inside the virtual network.
	apiAddr := fmt.Sprintf("%s:80", cfg.GatewayIP)
	vnLn, err := vn.Listen("tcp", apiAddr)
	if err != nil {
		logger.Error(err, "Failed to listen on API address", "addr", apiAddr)
		return
	}

Fix: Add explicit ln.Close() to each early-return path after line 96:

 	cfg := newVirtualNetworkConfig(*subnet)
 	vn, err := virtualnetwork.New(&cfg)
 	if err != nil {
+		ln.Close()
 		logger.Error(err, "Failed to create virtual network")
 		return
 	}
 ...
 	vnLn, err := vn.Listen("tcp", apiAddr)
 	if err != nil {
+		ln.Close()
 		logger.Error(err, "Failed to listen on API address", "addr", apiAddr)
 		return
 	}