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 }