From 012516033facad74e1571c8db669bf79b1e3f695 Mon Sep 17 00:00:00 2001 From: Dawei Wei Date: Thu, 23 Jul 2026 18:15:21 +0000 Subject: [PATCH] shim: don't tear down shared pod UVM on Hyper-V container restart On Hyper-V-isolated Windows pods (runhcs-wcow-hypervisor), in-place container restarts (ContainerRestartRules / RestartAllContainersOnContainerExits) fail: when a workload container exits it is not restarted in place; the pod ends up Failed. Two hcsshim/GCS issues combine to destroy the shared pod UVM. 1. Under RestartAll churn the guest GCS delivers a container's WaitForProcess exit reply late (or drops it); a crossing SIGKILL then returns hrNotFound while the wait is still pending. Process.Signal only logged "ignoring missing process", so the wait never completed and container Stop blocked forever. Force-complete the pending wait on hrNotFound so Wait()/Stop can't hang, and make an unmatched late response in recvLoop non-fatal (log and continue) rather than tearing the bridge down. 2. The KillExec 30s SIGKILL-init watchdog closed ht.host directly with no ownsHost guard, so a non-owning workload container's stuck stop tore down the shared pod UVM, killing the sandbox and its siblings. Gate the watchdog on ht.ownsHost and route teardown through closeHost so only the UVM owner may close it. Process-isolated pods are unaffected (host HCS exit path, no shared UVM). Adds unit tests for bridge.forceComplete and the non-fatal unknown-rpc path. Signed-off-by: Dawei Wei --- cmd/containerd-shim-runhcs-v1/task_hcs.go | 17 ++++---- internal/gcs/bridge.go | 25 ++++++++++- internal/gcs/bridge_test.go | 52 +++++++++++++++++++++++ internal/gcs/process.go | 5 ++- 4 files changed, 89 insertions(+), 10 deletions(-) diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs.go b/cmd/containerd-shim-runhcs-v1/task_hcs.go index afadc50c5e..aec4d8945a 100644 --- a/cmd/containerd-shim-runhcs-v1/task_hcs.go +++ b/cmd/containerd-shim-runhcs-v1/task_hcs.go @@ -469,11 +469,11 @@ func (ht *hcsTask) KillExec(ctx context.Context, eid string, signal uint32, all return true }) } - if signal == 0x9 && eid == "" && ht.host != nil { - // 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. + if signal == 0x9 && eid == "" && ht.host != nil && ht.ownsHost { + // SIGKILL to a UVM-owning task's init process: watchdog the guest and + // force-close the UVM if it doesn't exit in time. Gated on ownsHost so a + // workload container sharing the pod UVM can't tear down the sandbox and + // break in-place container restarts on Hyper-V pods. go func() { t := time.NewTimer(30 * time.Second) execExited := make(chan struct{}) @@ -485,9 +485,10 @@ func (ht *hcsTask) KillExec(ctx context.Context, eid string, signal uint32, all case <-execExited: t.Stop() case <-t.C: - // Safe to call multiple times if called previously on - // successful shutdown. - ht.host.Close() + log.G(ctx).WithField("tid", ht.id).Warn( + "hcsTask::KillExec watchdog expired; force-closing owned UVM") + // closeHost honors the ownsHost guard and emits TaskExit. + ht.closeHost(ctx) } }() } diff --git a/internal/gcs/bridge.go b/internal/gcs/bridge.go index b18112e624..5fa4ea2213 100644 --- a/internal/gcs/bridge.go +++ b/internal/gcs/bridge.go @@ -221,6 +221,22 @@ func (call *rpc) complete(err error) { close(call.ch) } +// forceComplete completes a still-pending RPC out of band when the guest will +// never send its response (e.g. a signal reported the process missing). It +// removes the call from the map under lock first so a late response can't +// double-complete it; returns false if the RPC is no longer tracked. +func (brdg *bridge) forceComplete(call *rpc, err error) bool { + brdg.mu.Lock() + if _, ok := brdg.rpcs[call.id]; !ok { + brdg.mu.Unlock() + return false + } + delete(brdg.rpcs, call.id) + brdg.mu.Unlock() + call.complete(err) + return true +} + type rpcError struct { result int32 message string @@ -389,7 +405,14 @@ func (brdg *bridge) recvLoop() error { delete(brdg.rpcs, id) brdg.mu.Unlock() if call == nil { - return fmt.Errorf("bridge received unknown rpc response for id %d, type %s", id, typ) + // No pending call: force-completed out of band and the guest's + // real response arrived late. Dropping it (vs. fatal) avoids + // tearing down the shared pod UVM. + brdg.log.WithFields(logrus.Fields{ + "message-id": id, + "type": typ.String(), + }).Warning("bridge received response for unknown rpc id; ignoring") + continue } err := json.Unmarshal(b, call.resp) if err != nil { diff --git a/internal/gcs/bridge_test.go b/internal/gcs/bridge_test.go index fcd9a55ea2..ca0c58194d 100644 --- a/internal/gcs/bridge_test.go +++ b/internal/gcs/bridge_test.go @@ -256,3 +256,55 @@ func TestRPCErrorUnwrapHCSCode(t *testing.T) { t.Fatalf("hcs.IsNotExist(wrapped) = false; want true (err=%v)", wrapped) } } + +func TestBridgeForceComplete(t *testing.T) { + s, _ := pipeConn() + b := newBridge(s, nil, logrus.NewEntry(logrus.StandardLogger())) + + call := &rpc{ch: make(chan struct{}), id: 42} + b.rpcs[call.id] = call + + sentinel := errors.New("forced") + if !b.forceComplete(call, sentinel) { + t.Fatal("forceComplete should report true for a tracked rpc") + } + if !call.Done() { + t.Fatal("rpc should be completed after forceComplete") + } + if !errors.Is(call.Err(), sentinel) { + t.Fatalf("expected err %v, got %v", sentinel, call.Err()) + } + if _, ok := b.rpcs[call.id]; ok { + t.Fatal("rpc should be removed from the tracking map") + } + + // A second call is a no-op: the rpc is no longer tracked. + if b.forceComplete(call, nil) { + t.Fatal("forceComplete on an untracked rpc should report false") + } +} + +func TestBridgeRecvUnknownRPCResponseIsNonFatal(t *testing.T) { + s, c := pipeConn() + b := newBridge(s, nil, logrus.NewEntry(logrus.StandardLogger())) + b.Start() + defer b.Close() + + go func() { + // Response for an id that was never requested (as when a call was + // force-completed and the guest's real response arrives late). + sendMessage(t, c, prot.MsgType(prot.RPCCreate)|prot.MsgTypeResponse, 99999, []byte("{}")) + // Reflect so a subsequent real RPC can still complete. + reflector(t, c, 0) + }() + + // The bridge must still be usable after the unknown-id response. + req := testReq{X: 7} + var resp testResp + if err := b.RPC(context.Background(), prot.RPCCreate, &req, &resp, false); err != nil { + t.Fatalf("bridge should survive an unknown-id response, got: %v", err) + } + if resp.X != req.X { + t.Fatalf("expected echoed X=%d, got %d", req.X, resp.X) + } +} diff --git a/internal/gcs/process.go b/internal/gcs/process.go index c4d29639f5..56917ae73a 100644 --- a/internal/gcs/process.go +++ b/internal/gcs/process.go @@ -287,7 +287,10 @@ 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; synthesizing exit to unblock wait") + // Guest reported the process gone but never delivered its exit; + // force-complete the wait so Wait()/Stop don't block forever. + p.gc.brdg.forceComplete(p.waitCall, nil) } return false, nil }