Subprocess: Use pidfd (PIDFD_GET_INFO if supported) to monitor an imported process for exit status

lxd/subprocess/bgpm_test.go

@@ -238,14 +238,9 @@ func savePidFile(t *testing.T, p *Process, mutate func(*Process)) string {
 
 	mutate(saved)
 
-	dat, err = yaml.Marshal(saved)
+	err = saved.Save(path)
 	if err != nil {
-		t.Fatal("Failed serializing pid file: ", err)
-	}
-
-	err = os.WriteFile(path, dat, 0600)
-	if err != nil {
-		t.Fatal("Failed writing pid file: ", err)
+		t.Fatal("Failed saving mutated pid file: ", err)
 	}
 
 	return path
@@ -410,3 +405,61 @@ func TestImportOldFormat(t *testing.T) {
 		t.Error("Failed stopping process imported from old-format file: ", err)
 	}
 }
+
+// TestImportWaitExitCode checks that a live process imported from a pid file is waited on via a
+// pidfd (rather than as a child), that Wait blocks until it exits, and that its exit code is
+// reported when the kernel supports PIDFD_GET_INFO.
+func TestImportWaitExitCode(t *testing.T) {
+	// Start a process that stays alive briefly then exits with a known non-zero code.
+	p, err := NewProcess("sh", []string{"-c", "sleep 0.5; exit 42"}, "", "")
+	if err != nil {
+		t.Fatal("Failed process creation: ", err)
+	}
+
+	err = p.Start(context.Background())
+	if err != nil {
+		t.Fatal("Failed starting process: ", err)
+	}
+
+	t.Cleanup(func() { _ = p.Stop() })
+
+	path := savePidFile(t, p, nil)
+
+	// Import while the process is still running so a pidfd-based monitor is started.
+	imp, err := ImportProcess(path)
+	if err != nil {
+		t.Fatal("Failed importing process: ", err)
+	}
+
+	if !imp.hasMonitor {
+		t.Fatal("Imported live process should have a monitor to be waited on")
+	}
+
+	// Wait must block on the pidfd until the process exits, then report its exit code.
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	started := time.Now()
+	code, err := imp.Wait(ctx)
+	if errors.Is(err, context.DeadlineExceeded) {
+		t.Fatal("Wait on imported process timed out")
+	}
+
+	if time.Since(started) < 300*time.Millisecond {
+		t.Error("Wait returned before the process exited; it did not block on the pidfd")
+	}
+
+	if code == -1 {
+		// The kernel lacks PIDFD_GET_INFO exit support; the wait still completed on exit.
+		t.Log("Kernel does not support PIDFD_GET_INFO; exit code unavailable")
+		return
+	}
+
+	if code != 42 {
+		t.Errorf("Expected exit code 42 but got %d", code)
+	}
+
+	if err == nil {
+		t.Error("Expected a non-nil error for a non-zero exit code")
+	}
+}

lxd/subprocess/manager.go

@@ -94,5 +94,12 @@ func ImportProcess(path string) (*Process, error) {
 		}
 	}
 
+	// Spawn a monitor goroutine so a running imported process can be waited on like a spawned
+	// one. Only do so when the process is actually alive; the monitor records the exit code and
+	// starting it for an already-exited process would race with any later reuse of the object.
+	if proc.Signal(0) == nil {
+		proc.monitorImported()
+	}
+
 	return &proc, nil
 }

lxd/subprocess/proc.go

@@ -85,6 +85,44 @@ func (p *Process) finish() {
 	p.release()
 }
 
+// startMonitor spawns a goroutine that waits for the process to exit using the supplied wait
+// function, records the resulting exit code and error, and closes chExit. It is shared by spawned
+// processes (which reap via the child handle) and imported processes (which wait via a pidfd).
+func (p *Process) startMonitor(wait func() (int64, error)) {
+	p.exitCode = -1
+	p.exitErr = nil
+	chExit := make(chan struct{})
+	p.chExit = chExit
+	p.hasMonitor = true
+
+	go func() {
+		defer close(chExit)
+
+		p.exitCode, p.exitErr = wait()
+	}()
+}
+
+// monitorImported starts a monitor for an imported (non-child) process, waiting on it via a pidfd.
+// On kernels that support PIDFD_GET_INFO the exit code is recorded, otherwise it is unknown.
+func (p *Process) monitorImported() {
+	// Capture the identity locally so the wait does not touch p, which may be reused (e.g. via
+	// Start or Restart) while this monitor is still running.
+	pid := p.PID
+	startTime := p.StartTime
+	p.startMonitor(func() (int64, error) {
+		code, err := waitProcess(context.Background(), pid, startTime)
+		if err != nil {
+			return -1, nil
+		}
+
+		if code > 0 {
+			return code, fmt.Errorf("Process exited with non-zero value %d", code)
+		}
+
+		return code, nil
+	})
+}
+
 // Stop will stop the given process object.
 func (p *Process) Stop() error {
 	if p.proc == nil {
@@ -174,34 +212,25 @@ func (p *Process) start(ctx context.Context, fds []*os.File) error {
 	if err == nil {
 		p.BootID = bootID
 	}
-	// Reset exitCode/exitErr
-	p.exitCode = 0
-	p.exitErr = nil
-
-	// Spawn a goroutine waiting for it to exit.
-	p.chExit = make(chan struct{})
-	p.hasMonitor = true
-	go func() {
-		defer close(p.chExit)
 
+	p.startMonitor(func() (int64, error) {
 		err := cmd.Wait()
 
+		code := int64(-1)
 		if cmd.ProcessState != nil {
-			p.exitCode = int64(cmd.ProcessState.ExitCode())
-		} else {
-			p.exitCode = -1
+			code = int64(cmd.ProcessState.ExitCode())
 		}
 
 		if err != nil {
-			p.exitErr = err
-
-			return
+			return code, err
 		}
 
-		if p.exitCode != 0 {
-			p.exitErr = fmt.Errorf("Process exited with non-zero value %d", p.exitCode)
+		if code != 0 {
+			return code, fmt.Errorf("Process exited with non-zero value %d", code)
 		}
-	}()
+
+		return code, nil
+	})
 
 	return nil
 }
@@ -242,7 +271,23 @@ func (p *Process) Reload() error {
 
 // Save will save the given process object to a YAML file. Can be imported at a later point.
 func (p *Process) Save(path string) error {
-	dat, err := yaml.Marshal(p)
+	// Marshal a copy of only the persisted fields. Marshalling the live process would have yaml
+	// read the whole struct (via reflect) and race with the monitor goroutine updating the
+	// exit state. The excluded fields are not persisted anyway.
+	saved := Process{
+		Name:        p.Name,
+		Args:        p.Args,
+		Apparmor:    p.Apparmor,
+		PID:         p.PID,
+		BootID:      p.BootID,
+		UID:         p.UID,
+		GID:         p.GID,
+		SetGroups:   p.SetGroups,
+		StartTime:   p.StartTime,
+		SysProcAttr: p.SysProcAttr,
+	}
+
+	dat, err := yaml.Marshal(&saved)
 	if err != nil {
 		return fmt.Errorf("Cannot serialize process struct to YAML: %w", err)
 	}

lxd/subprocess/procwait_linux.go

@@ -0,0 +1,119 @@
+//go:build linux
+
+package subprocess
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"syscall"
+	"unsafe"
+
+	"golang.org/x/sys/unix"
+)
+
+// pidfdInfo mirrors the kernel struct pidfd_info used by the PIDFD_GET_INFO ioctl.
+type pidfdInfo struct {
+	mask           uint64
+	cgroupid       uint64
+	pid            uint32
+	tgid           uint32
+	ppid           uint32
+	ruid           uint32
+	rgid           uint32
+	euid           uint32
+	egid           uint32
+	suid           uint32
+	sgid           uint32
+	fsuid          uint32
+	fsgid          uint32
+	exitCode       int32
+	coredumpMask   uint32
+	coredumpSignal uint32
+	coredumpCode   uint32
+	coredumpPad    uint32
+	supportedMask  uint64
+}
+
+const (
+	// pidfdInfoExit requests exit information from PIDFD_GET_INFO (Linux 6.15+).
+	pidfdInfoExit = 1 << 3
+
+	// pidfsIoctlMagic and iocDirWriteRead build the PIDFD_GET_INFO request number
+	// (_IOWR(0xFF, 11, struct pidfd_info)).
+	pidfsIoctlMagic = 0xff
+	iocDirWriteRead = 3
+)
+
+// waitProcess blocks until the process identified by pid has exited, or ctx is cancelled.
+// It uses a pidfd so it can wait on a process that is not a child of the current process
+// (for example one imported from a PID file). The startTime is used to guard against PID reuse.
+// On kernels that support PIDFD_GET_INFO it returns the process exit code, otherwise -1.
+func waitProcess(ctx context.Context, pid int, startTime int64) (int64, error) {
+	pidFd, err := unix.PidfdOpen(pid, 0)
+	if err != nil {
+		// ESRCH means the process is already gone.
+		if errors.Is(err, unix.ESRCH) {
+			return -1, nil
+		}
+
+		return -1, fmt.Errorf("Failed opening pidfd for PID %d: %w", pid, err)
+	}
+
+	defer func() { _ = unix.Close(pidFd) }()
+
+	// Guard against PID reuse: if the recorded start time no longer matches, the original
+	// process has already exited and a new one has reused its PID.
+	if startTime != 0 {
+		currentStartTime, err := processStartTime(pid)
+		if err != nil || currentStartTime != startTime {
+			return -1, nil
+		}
+	}
+
+	// The pidfd becomes readable once the process exits.
+	pollFds := []unix.PollFd{{Fd: int32(pidFd), Events: unix.POLLIN}}
+	for {
+		// Poll with a bounded timeout so context cancellation can be observed.
+		n, err := unix.Poll(pollFds, 100)
+		if err != nil {
+			if errors.Is(err, unix.EINTR) {
+				continue
+			}
+
+			return -1, fmt.Errorf("Failed polling pidfd for PID %d: %w", pid, err)
+		}
+
+		if n > 0 {
+			return pidfdExitCode(pidFd), nil
+		}
+
+		if ctx.Err() != nil {
+			return -1, ctx.Err()
+		}
+	}
+}
+
+// pidfdExitCode returns the exit code of the exited process referenced by pidFd using the
+// PIDFD_GET_INFO ioctl, or -1 if the kernel does not support it or the process was signalled.
+func pidfdExitCode(pidFd int) int64 {
+	info := pidfdInfo{mask: pidfdInfoExit}
+	req := (uintptr(iocDirWriteRead) << 30) | (unsafe.Sizeof(info) << 16) | (uintptr(pidfsIoctlMagic) << 8) | 11
+
+	_, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(pidFd), req, uintptr(unsafe.Pointer(&info)))
+	if errno != 0 {
+		return -1
+	}
+
+	// The kernel clears the requested bit if it could not provide the information.
+	if info.mask&pidfdInfoExit == 0 {
+		return -1
+	}
+
+	waitStatus := syscall.WaitStatus(info.exitCode)
+	if !waitStatus.Exited() {
+		return -1
+	}
+
+	return int64(waitStatus.ExitStatus())
+}

lxd/subprocess/procwait_other.go

@@ -0,0 +1,34 @@
+//go:build !linux && !windows
+
+package subprocess
+
+import (
+	"context"
+	"os"
+	"syscall"
+	"time"
+)
+
+// waitProcess blocks until the process identified by pid has exited, or ctx is cancelled.
+// pidfd is only available on Linux, so this fallback polls the process for liveness and cannot
+// determine the exit code, which is reported as -1.
+func waitProcess(ctx context.Context, pid int, startTime int64) (int64, error) {
+	proc, err := os.FindProcess(pid)
+	if err != nil {
+		return -1, nil
+	}
+
+	for {
+		// Signal 0 reports whether the process is still present.
+		err := proc.Signal(syscall.Signal(0))
+		if err != nil {
+			return -1, nil
+		}
+
+		select {
+		case <-ctx.Done():
+			return -1, ctx.Err()
+		case <-time.After(50 * time.Millisecond):
+		}
+	}
+}