Deadlock Patch

cmd/agent/listen_linux.go

@@ -219,8 +219,7 @@ type virtioSerialConn struct {
 
 // instrumentVirtioSerial controls per-Read logging on the conn. Set true via
 // OSB_AGENT_TRACE_VIRTIO=1 env var to debug post-loadvm protocol confusion.
-// HARDCODED TO TRUE FOR INVESTIGATION BUILD — revert before merging.
-var instrumentVirtioSerial = true || os.Getenv("OSB_AGENT_TRACE_VIRTIO") == "1"
+var instrumentVirtioSerial = os.Getenv("OSB_AGENT_TRACE_VIRTIO") == "1"
 
 func (c *virtioSerialConn) Read(b []byte) (int, error) {
 	n, err := c.f.Read(b)

internal/agent/stats.go

@@ -228,6 +228,35 @@ func (s *Server) PrepareHibernate(ctx context.Context, req *pb.PrepareHibernateR
 	return &pb.PrepareHibernateResponse{}, nil
 }
 
+// defaultThawMounts are unfrozen when a Thaw request carries no explicit list.
+// Mirrors the snapshot/hibernate freeze set (rootfs + workspace). In the merged
+// layout /home/sandbox is a directory on the rootfs, so its FITHAW hits the same
+// (already-thawed) superblock and returns EINVAL → already_thawed, harmlessly.
+var defaultThawMounts = []string{"/", "/home/sandbox"}
+
+// Thaw unfreezes guest filesystems that the snapshot/hibernate path froze, using
+// a native FITHAW ioctl instead of exec'ing `fsfreeze --unfreeze`. Native is
+// load-bearing here: the golden/hibernate snapshot is captured with the rootfs
+// frozen, and exec'ing a binary from that frozen rootfs to thaw it can deadlock
+// (exec-needs-fs, fs-needs-thaw). See thawMount. Idempotent — a fs that is not
+// frozen is reported as already_thawed, not an error.
+func (s *Server) Thaw(ctx context.Context, req *pb.ThawRequest) (*pb.ThawResponse, error) {
+	mounts := req.Mountpoints
+	if len(mounts) == 0 {
+		mounts = defaultThawMounts
+	}
+	resp := &pb.ThawResponse{}
+	for _, mp := range mounts {
+		already, err := thawMount(mp)
+		res := &pb.ThawResult{Mountpoint: mp, AlreadyThawed: already, Thawed: err == nil && !already}
+		if err != nil {
+			res.Error = err.Error()
+		}
+		resp.Results = append(resp.Results, res)
+	}
+	return resp, nil
+}
+
 // flushBlockDevices issues BLKFLSBUF on each device (equivalent to `blockdev --flushbufs`).
 // Ignores errors — not all devices may be present (e.g., /dev/vdb).
 func flushBlockDevices(paths ...string) {

internal/agent/thaw_linux.go

@@ -0,0 +1,44 @@
+//go:build linux
+
+package agent
+
+import (
+	"fmt"
+	"syscall"
+)
+
+// fithawIoctl is FITHAW = _IOWR('X', 120, int) — unfreeze a filesystem.
+// (FIFREEZE is 0xc0045877.) We hardcode it because the syscall package does not
+// export it, mirroring flushBlockDevices' use of a literal BLKFLSBUF constant.
+const fithawIoctl = 0xc0045878
+
+// thawMount unfreezes the filesystem containing mountpoint via the FITHAW ioctl.
+//
+// This is deliberately native — no exec. The golden/hibernate snapshot is
+// captured with the rootfs fsfrozen, and thawing it by exec'ing a binary that
+// lives on that frozen rootfs can deadlock (exec faults/atime take
+// sb_start_write(), which blocks until thaw — which needs the exec). Opening the
+// mountpoint O_RDONLY|O_NOATIME and issuing FITHAW never writes to the fs:
+//   - O_NOATIME suppresses the access-time update that a normal open/exec would
+//     perform (the operation that takes sb_start_write() and blocks on a frozen fs);
+//   - a read-only directory open and the FITHAW ioctl itself modify nothing.
+// So this is safe to call against a still-frozen filesystem, which is the whole
+// point. The agent runs as root, so O_NOATIME is permitted.
+//
+// Returns alreadyThawed=true when the fs was not frozen (FITHAW → EINVAL), which
+// we treat as success so the call is idempotent across wake/fork/golden-restore.
+func thawMount(mountpoint string) (alreadyThawed bool, err error) {
+	fd, err := syscall.Open(mountpoint, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOATIME|syscall.O_CLOEXEC, 0)
+	if err != nil {
+		return false, fmt.Errorf("open %s: %w", mountpoint, err)
+	}
+	defer syscall.Close(fd)
+
+	if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), uintptr(fithawIoctl), 0); errno != 0 {
+		if errno == syscall.EINVAL {
+			return true, nil // not frozen — already thawed
+		}
+		return false, fmt.Errorf("FITHAW %s: %w", mountpoint, errno)
+	}
+	return false, nil
+}

internal/agent/thaw_linux_test.go

@@ -0,0 +1,27 @@
+//go:build linux
+
+package agent
+
+import (
+	"os"
+	"testing"
+)
+
+// TestThawMount_UnfrozenIsIdempotent exercises the real FITHAW ioctl against an
+// un-frozen filesystem: thaw_super() returns EINVAL, which thawMount maps to
+// alreadyThawed=true. This proves the ioctl constant, O_NOATIME open, and EINVAL
+// handling on real Linux without freezing (and thus risking) anything. Requires
+// CAP_SYS_ADMIN (FITHAW), so it skips when not run as root.
+func TestThawMount_UnfrozenIsIdempotent(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip("FITHAW requires CAP_SYS_ADMIN; run as root to exercise the ioctl")
+	}
+	dir := t.TempDir()
+	already, err := thawMount(dir)
+	if err != nil {
+		t.Fatalf("thawMount(%s) on an unfrozen fs: unexpected error %v", dir, err)
+	}
+	if !already {
+		t.Fatalf("thawMount(%s): want alreadyThawed=true for an unfrozen fs", dir)
+	}
+}

internal/agent/thaw_other.go

@@ -0,0 +1,11 @@
+//go:build !linux
+
+package agent
+
+import "fmt"
+
+// thawMount is a stub on non-Linux platforms (the agent only runs inside the
+// Linux guest; this exists so the package still builds on dev machines).
+func thawMount(mountpoint string) (alreadyThawed bool, err error) {
+	return false, fmt.Errorf("thaw not supported on this platform")
+}

internal/agent/thaw_test.go

@@ -0,0 +1,47 @@
+package agent
+
+import (
+	"context"
+	"testing"
+
+	pb "github.com/opensandbox/opensandbox/proto/agent"
+)
+
+// TestServerThaw_DefaultsMounts verifies an empty request thaws the agent's
+// default mount set (rootfs + workspace), one result per mountpoint.
+func TestServerThaw_DefaultsMounts(t *testing.T) {
+	s := &Server{}
+	resp, err := s.Thaw(context.Background(), &pb.ThawRequest{})
+	if err != nil {
+		t.Fatalf("Thaw returned error: %v", err)
+	}
+	if len(resp.Results) != len(defaultThawMounts) {
+		t.Fatalf("want %d results, got %d", len(defaultThawMounts), len(resp.Results))
+	}
+	for i, mp := range defaultThawMounts {
+		if resp.Results[i].Mountpoint != mp {
+			t.Errorf("result %d mountpoint = %q, want %q", i, resp.Results[i].Mountpoint, mp)
+		}
+	}
+}
+
+// TestServerThaw_ExplicitMountReported verifies explicit mountpoints are honored
+// and that an un-openable mountpoint is surfaced as a per-result error rather
+// than failing the whole RPC (best-effort semantics the host relies on).
+func TestServerThaw_ExplicitMountReported(t *testing.T) {
+	s := &Server{}
+	const missing = "/definitely/not/a/mountpoint/xyzzy"
+	resp, err := s.Thaw(context.Background(), &pb.ThawRequest{Mountpoints: []string{missing}})
+	if err != nil {
+		t.Fatalf("Thaw returned error: %v", err)
+	}
+	if len(resp.Results) != 1 {
+		t.Fatalf("want 1 result, got %d", len(resp.Results))
+	}
+	if resp.Results[0].Mountpoint != missing {
+		t.Errorf("mountpoint = %q, want %q", resp.Results[0].Mountpoint, missing)
+	}
+	if resp.Results[0].Error == "" {
+		t.Errorf("expected a per-result error for a non-existent mountpoint")
+	}
+}

internal/qemu/agent_client.go

@@ -224,6 +224,13 @@ func (c *AgentClient) PrepareHibernate(ctx context.Context, req *pb.PrepareHiber
 	return c.client.PrepareHibernate(ctx, req)
 }
 
+// Thaw unfreezes guest filesystems via the agent's native FITHAW ioctl (no exec
+// inside the guest). Empty req ⇒ the agent's default mount set. Old agents that
+// predate this RPC return codes.Unimplemented; callers fall back to the exec path.
+func (c *AgentClient) Thaw(ctx context.Context, req *pb.ThawRequest) (*pb.ThawResponse, error) {
+	return c.client.Thaw(ctx, req)
+}
+
 // ReadFile reads a file from the VM.
 func (c *AgentClient) ReadFile(ctx context.Context, path string) ([]byte, error) {
 	resp, err := c.client.ReadFile(ctx, &pb.ReadFileRequest{Path: path})

internal/qemu/manager.go

@@ -837,6 +837,44 @@ func (m *Manager) agentFsThawAfterWake(ctx context.Context, sandboxID string, ag
 	if agent == nil {
 		return
 	}
+	// Native FITHAW ioctl inside the guest — NOT `fsfreeze --unfreeze` via Exec.
+	// The snapshot is captured with the rootfs frozen; thawing it by exec'ing a
+	// binary that lives on that frozen rootfs is a latent deadlock (exec-needs-fs,
+	// fs-needs-thaw) that intermittently stalls every post-restore Exec RPC
+	// (network patch, clock sync, logship) until the create deadline. The native
+	// Thaw RPC opens the mountpoint O_NOATIME and ioctls it — no exec, no write to
+	// the frozen fs — so it cannot deadlock. See internal/agent thawMount.
+	thawCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+	defer cancel()
+	resp, err := agent.Thaw(thawCtx, &pb.ThawRequest{})
+	if err == nil {
+		for _, r := range resp.GetResults() {
+			if r.GetError() != "" {
+				log.Printf("qemu: %s: native thaw %s: %s", sandboxID, r.GetMountpoint(), r.GetError())
+			}
+		}
+		return
+	}
+	if status.Code(err) == codes.Unimplemented {
+		// Agent predates the Thaw RPC (mixed fleet mid-rollout). Fall back to the
+		// legacy exec thaw — the very path with the deadlock hazard, but it's the
+		// best an old agent can do. Dead once every worker's golden is rebuilt on
+		// the new agent.
+		log.Printf("qemu: %s: agent has no native Thaw (Unimplemented), using exec fallback", sandboxID)
+		m.agentFsThawViaExec(ctx, sandboxID, agent)
+		return
+	}
+	// Other errors (e.g. transport): best-effort exec fallback + loud log, since
+	// an un-thawed fs hangs customer writes.
+	log.Printf("qemu: %s: native Thaw failed: %v — trying exec fallback", sandboxID, err)
+	m.agentFsThawViaExec(ctx, sandboxID, agent)
+}
+
+// agentFsThawViaExec is the legacy exec-based thaw, retained only as a rollout
+// fallback for agents that predate the native Thaw RPC. It carries the
+// exec-on-frozen-fs deadlock hazard that native Thaw exists to remove — do not
+// add new call sites; call agentFsThawAfterWake instead.
+func (m *Manager) agentFsThawViaExec(ctx context.Context, sandboxID string, agent *AgentClient) {
 	execCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
 	defer cancel()
 	if _, err := agent.Exec(execCtx, &pb.ExecRequest{