shim: don't tear down shared pod UVM on Hyper-V container restart

cmd/containerd-shim-runhcs-v1/task_hcs.go

@@ -469,11 +469,14 @@ func (ht *hcsTask) KillExec(ctx context.Context, eid string, signal uint32, all
 			return true
 		})
 	}
-	if signal == 0x9 && eid == "" && ht.host != nil {
+	if signal == 0x9 && eid == "" && ht.host != nil && ht.ownsHost {
 		// If this is a SIGKILL against the init process we start a background
 		// timer and wait on either the timer expiring or the process exiting
 		// cleanly. If the timer expires first we forcibly close the UVM as we
 		// assume the guest is misbehaving for some reason.
+		//
+		// Only a task that owns its UVM may close it. A workload container
+		// shares the pod UVM, and closing it would tear down the whole pod.
 		go func() {
 			t := time.NewTimer(30 * time.Second)
 			execExited := make(chan struct{})

internal/gcs/process.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"sync"
+	"time"
 
 	"github.com/Microsoft/go-winio"
 	"github.com/Microsoft/hcsshim/internal/cow"
@@ -30,6 +31,12 @@ const (
 	exitProbeTimeoutMs = 50
 )
 
+// missingProcessExitGrace is how long releaseMissingProcessExit waits for the
+// real exit before closing the stdio relay. It matches the default
+// CopyAfterExitTimeout the host allows stdio relays after a process exits.
+// A variable so tests can shorten it.
+var missingProcessExitGrace = time.Second
+
 // Process represents a process in a container or container host.
 type Process struct {
 	gc                    *GuestConnection
@@ -43,6 +50,8 @@ type Process struct {
 	// stdinPort, stdoutPort, stderrPort are the vsock ports gc.exec
 	// allocated for this process's stdio relay.
 	stdinPort, stdoutPort, stderrPort uint32
+	// missingOnce starts releaseMissingProcessExit at most once.
+	missingOnce sync.Once
 }
 
 var _ cow.Process = &Process{}
@@ -294,13 +303,47 @@ func (p *Process) Signal(ctx context.Context, options interface{}) (_ bool, err
 				logrus.ErrorKey:       err,
 				logfields.ContainerID: p.cid,
 				logfields.ProcessID:   p.id,
-			}).Warn("ignoring missing process")
+			}).Warn("process reported missing by guest while its wait is pending")
+			// gc.os is the OS of the guest (the UVM), not of the host. Only a
+			// Windows guest (WCOW) holds the wait response until the host
+			// closes the stdio relay. A Linux guest (LCOW) drains and closes
+			// the relay itself, and reconnects if the host closes it first.
+			if p.gc.os == "windows" {
+				p.missingOnce.Do(func() {
+					go p.releaseMissingProcessExit(context.WithoutCancel(ctx))
+				})
+			}
 		}
 		return false, nil
 	}
 	return true, nil
 }
 
+// releaseMissingProcessExit closes the host side of the stdio relay of a
+// process the guest reported missing while its wait is still pending.
+//
+// A Windows guest holds the wait response of a process until the host closes
+// its stdio relay, and a descendant process can keep the relay open after the
+// process itself exits. The host only closes the relay once the wait
+// completes, so neither side would make progress. The real exit normally
+// follows the missing report closely, so give it a short grace period first.
+func (p *Process) releaseMissingProcessExit(ctx context.Context) {
+	t := time.NewTimer(missingProcessExitGrace)
+	defer t.Stop()
+	select {
+	case <-p.waitCall.ch:
+		return
+	case <-t.C:
+	}
+	log.G(ctx).WithFields(logrus.Fields{
+		logfields.ContainerID: p.cid,
+		logfields.ProcessID:   p.id,
+	}).Warn("closing stdio relay of process reported missing by guest")
+	_ = p.stdin.Close()
+	_ = p.stdout.Close()
+	_ = p.stderr.Close()
+}
+
 // Stdio returns the standard IO streams associated with the container. They
 // will be closed when Close is called.
 func (p *Process) Stdio() (stdin io.Writer, stdout, stderr io.Reader) {

internal/gcs/process_test.go

@@ -0,0 +1,218 @@
+//go:build windows
+
+package gcs
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/Microsoft/hcsshim/internal/gcs/prot"
+	"github.com/sirupsen/logrus"
+)
+
+func setMissingProcessExitGrace(t *testing.T, grace time.Duration) {
+	t.Helper()
+	old := missingProcessExitGrace
+	missingProcessExitGrace = grace
+	t.Cleanup(func() { missingProcessExitGrace = old })
+}
+
+// newMissingProcessTest returns a started bridge with a process whose
+// WaitForProcess is pending, and the guest side of the bridge connection.
+func newMissingProcessTest(t *testing.T, guestOS string) (*Process, io.ReadWriteCloser) {
+	t.Helper()
+	s, c := pipeConn()
+	b := newBridge(s, nil, logrus.NewEntry(logrus.StandardLogger()))
+	p := &Process{
+		gc:       &GuestConnection{brdg: b, os: guestOS},
+		cid:      t.Name(),
+		id:       1068,
+		waitResp: &prot.ContainerWaitForProcessResponse{},
+	}
+	p.waitCall = &rpc{
+		ch:   make(chan struct{}),
+		id:   42,
+		proc: prot.RPCWaitForProcess,
+		resp: p.waitResp,
+	}
+	b.rpcs[p.waitCall.id] = p.waitCall
+	b.nextID = p.waitCall.id + 1
+	b.Start()
+	t.Cleanup(func() { b.Close() })
+	return p, c
+}
+
+// replySignalNotFound reads the SignalProcess request and answers hrNotFound.
+func replySignalNotFound(t *testing.T, c io.ReadWriter) {
+	t.Helper()
+	signalID, signalType, _, err := readMessage(c)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	if got := signalType &^ prot.MsgTypeRequest; got != prot.MsgType(prot.RPCSignalProcess) {
+		t.Errorf("request type = %s, want SignalProcess", signalType)
+		return
+	}
+	result := uint32(hrNotFound)
+	resp, err := json.Marshal(&prot.ResponseBase{
+		Result:       int32(result),
+		ErrorMessage: "Element not found.",
+	})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	sendMessage(t, c, signalType^prot.MsgTypeRequest^prot.MsgTypeResponse, signalID, resp)
+}
+
+func sendWaitResponse(t *testing.T, c io.Writer, id int64, exitCode uint32) {
+	t.Helper()
+	resp, err := json.Marshal(&prot.ContainerWaitForProcessResponse{ExitCode: exitCode})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	sendMessage(t, c, prot.MsgType(prot.RPCWaitForProcess)|prot.MsgTypeResponse, id, resp)
+}
+
+func signalMissing(t *testing.T, p *Process) {
+	t.Helper()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	signaled, err := p.Signal(ctx, nil)
+	if err != nil {
+		t.Fatalf("Signal() error = %v", err)
+	}
+	if signaled {
+		t.Fatal("Signal() reported signaling a process the guest said was missing")
+	}
+}
+
+func waitForExit(t *testing.T, p *Process) int {
+	t.Helper()
+	select {
+	case <-p.waitCall.ch:
+	case <-time.After(10 * time.Second):
+		t.Fatal("pending WaitForProcess was not completed")
+	}
+	exitCode, err := p.ExitCode()
+	if err != nil {
+		t.Fatalf("ExitCode() error = %v", err)
+	}
+	return exitCode
+}
+
+// newStdioRelay returns a connected host-side relay channel and the guest end.
+func newStdioRelay(t *testing.T) (*ioChannel, net.Conn) {
+	t.Helper()
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	ch := newIoChannel(l)
+	guest, err := net.Dial("tcp", l.Addr().String())
+	if err != nil {
+		t.Fatal(err)
+	}
+	<-ch.ch
+	t.Cleanup(func() {
+		guest.Close()
+		ch.Close()
+	})
+	return ch, guest
+}
+
+// assertRelayOpen checks that the stdio relay still carries guest output.
+func assertRelayOpen(t *testing.T, host *ioChannel, guest net.Conn) {
+	t.Helper()
+	if _, err := guest.Write([]byte("x")); err != nil {
+		t.Fatalf("guest write to stdio relay: %v", err)
+	}
+	if err := host.c.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil {
+		t.Fatal(err)
+	}
+	b := make([]byte, 1)
+	if _, err := io.ReadFull(host, b); err != nil {
+		t.Fatalf("stdio relay was closed: %v", err)
+	}
+}
+
+// The guest may answer the signal before sending the wait response of a
+// process that just exited. The real exit code must be kept and the stdio
+// relay left open.
+func TestProcessSignalNotFoundKeepsRealExit(t *testing.T) {
+	const grace = 20 * time.Millisecond
+	setMissingProcessExitGrace(t, grace)
+	p, c := newMissingProcessTest(t, "windows")
+	var stdoutGuest net.Conn
+	p.stdout, stdoutGuest = newStdioRelay(t)
+	go func() {
+		replySignalNotFound(t, c)
+		sendWaitResponse(t, c, p.waitCall.id, 137)
+		reflector(t, c, 0)
+	}()
+
+	signalMissing(t, p)
+	if exitCode := waitForExit(t, p); exitCode != 137 {
+		t.Fatalf("ExitCode() = %d, want real exit code 137", exitCode)
+	}
+	time.Sleep(5 * grace)
+	assertRelayOpen(t, p.stdout, stdoutGuest)
+}
+
+// A Linux guest drains and closes the stdio relay itself before it answers
+// the wait, so the host must not close the relay for it.
+func TestProcessSignalNotFoundLinuxGuestKeepsStdio(t *testing.T) {
+	const grace = 20 * time.Millisecond
+	setMissingProcessExitGrace(t, grace)
+	p, c := newMissingProcessTest(t, "linux")
+	var stdoutGuest net.Conn
+	p.stdout, stdoutGuest = newStdioRelay(t)
+	signaled := make(chan struct{})
+	go func() {
+		replySignalNotFound(t, c)
+		<-signaled
+		sendWaitResponse(t, c, p.waitCall.id, 137)
+		reflector(t, c, 0)
+	}()
+
+	signalMissing(t, p)
+	time.Sleep(5 * grace)
+	assertRelayOpen(t, p.stdout, stdoutGuest)
+	close(signaled)
+	if exitCode := waitForExit(t, p); exitCode != 137 {
+		t.Fatalf("ExitCode() = %d, want real exit code 137", exitCode)
+	}
+}
+
+// A Windows guest holds the wait response of a process until the host closes
+// the process's stdio relay. Closing it must release the real exit code.
+func TestProcessSignalNotFoundClosesStdioToReleaseExit(t *testing.T) {
+	setMissingProcessExitGrace(t, 10*time.Millisecond)
+	p, c := newMissingProcessTest(t, "windows")
+	var stdoutGuest, stderrGuest net.Conn
+	p.stdout, stdoutGuest = newStdioRelay(t)
+	p.stderr, stderrGuest = newStdioRelay(t)
+	const exitCode = 0xC000013A
+	go func() {
+		replySignalNotFound(t, c)
+		for _, guest := range []net.Conn{stdoutGuest, stderrGuest} {
+			if _, err := io.Copy(io.Discard, guest); err != nil {
+				t.Error(err)
+				return
+			}
+		}
+		sendWaitResponse(t, c, p.waitCall.id, exitCode)
+		reflector(t, c, 0)
+	}()
+
+	signalMissing(t, p)
+	if got := waitForExit(t, p); got != exitCode {
+		t.Fatalf("ExitCode() = %#x, want real exit code %#x", got, exitCode)
+	}
+}