Skip to content

Commit 39d705b

Browse files
author
Shreyansh Sancheti
committed
shim: handle connection-closed errors during kill after live migration
After HCS live migration completes, FinalizeSourceLM tries to stop the source sandbox via containerd. The shim's Kill call fails with 'ttrpc: closed' because the VM has already departed and the connection (ttrpc transport or GCS bridge) is dead. This error propagates up as a StopSourceVMFailure, even though the VM is already gone. Three fixes: 1. exec_hcs: Treat ttrpc.ErrClosed and net.ErrClosed as successful kill. If the connection is dead, the process is effectively gone. The isConnectionClosed helper covers both the ttrpc transport path (containerd-to-shim) and the GCS bridge path (shim-to-guest vsock). 2. uvm: Make CloseCtx idempotent via sync.Once. Three goroutines can race to close the UVM (waitForHostExit, waitInitExit, KillExec timer). Without idempotency, the second caller panics on double-close of outputProcessingDone channel. 3. task_hcs: Route KillExec 30s timeout through closeHost instead of calling ht.host.Close() directly. The direct call bypassed closeHostOnce, skipping TaskExitEventTopic publish and ht.closed channel close. Fixes: #61773098 Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>
1 parent 25abbc6 commit 39d705b

4 files changed

Lines changed: 33 additions & 7 deletions

File tree

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ package main
44

55
import (
66
"context"
7+
stderrors "errors"
8+
"net"
79
"sync"
810
"time"
911

@@ -12,6 +14,7 @@ import (
1214
containerd_v1_types "github.com/containerd/containerd/api/types/task"
1315
"github.com/containerd/containerd/errdefs"
1416
"github.com/containerd/containerd/runtime"
17+
"github.com/containerd/ttrpc"
1518
"github.com/opencontainers/runtime-spec/specs-go"
1619
"github.com/pkg/errors"
1720
"github.com/sirupsen/logrus"
@@ -304,7 +307,7 @@ func (he *hcsExec) Kill(ctx context.Context, signal uint32) error {
304307
signalDelivered, deliveryErr := he.p.Process.Signal(ctx, options)
305308

306309
if deliveryErr != nil {
307-
if !hcs.IsAlreadyStopped(deliveryErr) {
310+
if !hcs.IsAlreadyStopped(deliveryErr) && !isConnectionClosed(deliveryErr) {
308311
// Process is not already stopped and there was a signal delivery error to this process
309312
log.G(ctx).WithField("err", deliveryErr).Errorf("Error in delivering signal %d, to pid: %d", signal, he.pid)
310313
}
@@ -328,6 +331,13 @@ func (he *hcsExec) Kill(ctx context.Context, signal uint32) error {
328331
// an already dead process.
329332
return nil
330333
}
334+
if isConnectionClosed(err) {
335+
// The connection to the compute system or GCS is closed.
336+
// This happens during live migration when the VM has already departed.
337+
// The process is effectively gone, so treat this as a successful kill.
338+
log.G(ctx).WithError(err).Warn("connection closed during kill; treating as successful")
339+
return nil
340+
}
331341
return err
332342
}
333343
if !delivered {
@@ -341,6 +351,13 @@ func (he *hcsExec) Kill(ctx context.Context, signal uint32) error {
341351
}
342352
}
343353

354+
// isConnectionClosed reports whether err indicates that the underlying connection
355+
// to the compute system or GCS is closed. This covers both the ttrpc transport
356+
// (containerd ↔ shim) and the GCS bridge (shim ↔ guest VM over vsock).
357+
func isConnectionClosed(err error) bool {
358+
return err == ttrpc.ErrClosed || stderrors.Is(err, net.ErrClosed)
359+
}
360+
344361
func (he *hcsExec) ResizePty(ctx context.Context, width, height uint32) error {
345362
he.sl.Lock()
346363
defer he.sl.Unlock()

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,9 +458,9 @@ func (ht *hcsTask) KillExec(ctx context.Context, eid string, signal uint32, all
458458
case <-execExited:
459459
t.Stop()
460460
case <-t.C:
461-
// Safe to call multiple times if called previously on
462-
// successful shutdown.
463-
ht.host.Close()
461+
// Route through closeHost to ensure proper event publishing and
462+
// that the close is guarded by closeHostOnce.
463+
ht.closeHost(ctx)
464464
}
465465
}()
466466
}

internal/uvm/create.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,19 @@ func (uvm *UtilityVM) Close() error { return uvm.CloseCtx(context.Background())
214214
//
215215
// The context is used for all operations, including waits, so timeouts/cancellations may prevent
216216
// proper uVM cleanup.
217-
func (uvm *UtilityVM) CloseCtx(ctx context.Context) (err error) {
217+
func (uvm *UtilityVM) CloseCtx(ctx context.Context) error {
218+
uvm.closeOnce.Do(func() {
219+
uvm.closeErr = uvm.closeCtxInternal(ctx)
220+
})
221+
return uvm.closeErr
222+
}
223+
224+
func (uvm *UtilityVM) closeCtxInternal(ctx context.Context) (err error) {
218225
ctx, span := oc.StartSpan(ctx, "uvm::Close")
219226
defer span.End()
220227
defer func() { oc.SetSpanStatus(span, err) }()
221228
span.AddAttributes(trace.StringAttribute(logfields.UVMID, uvm.id))
222229

223-
// TODO: check if uVM already closed
224-
225230
windows.Close(uvm.vmmemProcess)
226231

227232
if uvm.hcsSystem != nil {

internal/uvm/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,10 @@ type UtilityVM struct {
148148

149149
// confidentialUVMOptions hold confidential UVM specific options
150150
confidentialUVMOptions *ConfidentialOptions
151+
152+
// closeOnce ensures CloseCtx is idempotent and safe to call from multiple goroutines.
153+
closeOnce sync.Once
154+
closeErr error
151155
}
152156

153157
func (uvm *UtilityVM) ScratchEncryptionEnabled() bool {

0 commit comments

Comments
 (0)