Remove the hypervisor socket only after confirmed exit

shutdownHypervisor unlinked the control socket via a defer on every
return and again before waiting for the process to exit. On the paths
where the kill fails and standby resumes the VM, that left a live
hypervisor with no socket file, so a later graceful standby or stop
could not connect and fell straight into the force-kill path. Unlink
the socket only once the hypervisor is provably gone, matching the
contract killHypervisor already follows on delete and stop.

Author: yummybomb

lib/instances/process_identity_linux_test.go

@@ -243,6 +243,25 @@ func TestShutdownHypervisorSparesReusedPIDWhenNoProcessOwnsSocket(t *testing.T)
 	assert.NoError(t, syscall.Kill(pid, 0), "process with a recycled PID must not be killed")
 }
 
+func TestShutdownHypervisorRemovesStaleSocketWhenNoLiveOwner(t *testing.T) {
+	socketPath := filepath.Join(t.TempDir(), "stale.sock")
+	require.NoError(t, os.WriteFile(socketPath, nil, 0600))
+
+	// No process owns or references the socket and no client factory exists
+	// for this hypervisor type: the hypervisor is provably gone, so shutdown
+	// must report success and remove the stale socket file.
+	m := &manager{}
+	require.NoError(t, m.shutdownHypervisor(context.Background(), &Instance{
+		StoredMetadata: StoredMetadata{
+			Id:             "shutdown-stale-socket",
+			HypervisorType: hypervisor.Type("unregistered-stale-socket-test"),
+			SocketPath:     socketPath,
+		},
+	}))
+	_, statErr := os.Stat(socketPath)
+	assert.True(t, os.IsNotExist(statErr), "stale socket should be removed once the hypervisor is provably gone")
+}
+
 func TestClassifyResolvedHypervisorOwner(t *testing.T) {
 	const deadPID = 1<<22 - 1
 	require.False(t, ProcessExists(deadPID))

lib/instances/standby.go

@@ -368,11 +368,6 @@ func (m *manager) shutdownHypervisor(ctx context.Context, inst *Instance) error
 		return fmt.Errorf("confirm hypervisor ownership before shutdown: %w", err)
 	}
 
-	defer func() {
-		// Clean stale sockets even if graceful shutdown fails.
-		_ = os.Remove(inst.SocketPath)
-	}()
-
 	// Try to connect to hypervisor
 	hv, err := m.getHypervisor(inst.SocketPath, inst.HypervisorType)
 	if err != nil {
@@ -381,10 +376,13 @@ func (m *manager) shutdownHypervisor(ctx context.Context, inst *Instance) error
 			// alive; teardown is committed, so kill it rather than report a
 			// completed shutdown for a VMM that is still running.
 			log.WarnContext(ctx, "could not connect to hypervisor, force killing resolved owner", "instance_id", inst.Id, "pid", pid, "error", err)
-			return killProcessAndWait(pid)
+			if err := killProcessAndWait(pid); err != nil {
+				return err
+			}
 		}
-		// Can't connect - hypervisor might already be stopped
-		log.DebugContext(ctx, "could not connect to hypervisor, may already be stopped", "instance_id", inst.Id)
+		// The hypervisor is confirmed gone (killed above or no live owner);
+		// remove its stale socket.
+		_ = os.Remove(inst.SocketPath)
 		return nil
 	}
 
@@ -399,11 +397,6 @@ func (m *manager) shutdownHypervisor(ctx context.Context, inst *Instance) error
 		shutdownErr = hv.Shutdown(ctx)
 	}
 
-	// Teardown is committed; prevent new control-socket clients while the
-	// hypervisor exits. The deferred remove remains as a fallback for early
-	// returns above.
-	_ = os.Remove(inst.SocketPath)
-
 	// Wait for process to exit
 	if pid > 0 {
 		shouldWaitForGracefulExit := caps.SupportsGracefulVMMShutdown && shutdownErr != hypervisor.ErrNotSupported
@@ -425,8 +418,13 @@ func (m *manager) shutdownHypervisor(ctx context.Context, inst *Instance) error
 	}
 
 	// The hypervisor is confirmed gone (graceful exit, force kill, or no live
-	// owner). A graceful-API error at this point is not a failure: an error
-	// from this function means the hypervisor may still be running.
+	// owner), so its socket is stale now. Removing it any earlier would unlink
+	// the control socket of a VMM that survives the kill and gets resumed,
+	// leaving that VM unreachable for a later graceful standby or stop.
+	_ = os.Remove(inst.SocketPath)
+
+	// A graceful-API error at this point is not a failure: an error from this
+	// function means the hypervisor may still be running.
 	if shutdownErr != nil && shutdownErr != hypervisor.ErrNotSupported {
 		log.WarnContext(ctx, "graceful hypervisor shutdown failed, process force killed", "instance_id", inst.Id, "error", shutdownErr)
 	}