From 8d9468764e9c858385f46969356f28f66b15f1a6 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Fri, 20 Mar 2026 00:53:12 +0530 Subject: [PATCH] introduce lock in guestmanager to address concurrency at bottom layer Signed-off-by: Harsh Rawat --- internal/vm/guestmanager/guest.go | 16 ++++++++++++ internal/vm/guestmanager/manager.go | 27 +++++++++------------ internal/vm/guestmanager/request_helpers.go | 3 +++ 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/internal/vm/guestmanager/guest.go b/internal/vm/guestmanager/guest.go index c1d9656e3b..3fad8b0623 100644 --- a/internal/vm/guestmanager/guest.go +++ b/internal/vm/guestmanager/guest.go @@ -5,6 +5,7 @@ package guestmanager import ( "context" "fmt" + "sync" "github.com/Microsoft/hcsshim/internal/gcs" "github.com/Microsoft/hcsshim/internal/log" @@ -18,6 +19,10 @@ import ( // Guest manages the GCS connection and guest-side operations for a utility VM. type Guest struct { + // mu serializes all operations that interact with the guest connection (gc). + // This prevents parallel operations on the guest from racing on the GCS connection. + mu sync.RWMutex + log *logrus.Entry // uvm is the utility VM that this GuestManager is managing. // We restrict access to just lifetime manager and VMSocket manager. @@ -55,6 +60,14 @@ func WithInitializationState(state *gcs.InitialGuestState) ConfigOption { // CreateConnection accepts the GCS connection and performs initial setup. func (gm *Guest) CreateConnection(ctx context.Context, GCSServiceID guid.GUID, opts ...ConfigOption) error { + gm.mu.Lock() + defer gm.mu.Unlock() + + // Return early if a connection is already active. + if gm.gc != nil { + return nil + } + // The guest needs to connect to predefined GCS port. // The host must already be listening on these port before the guest attempts to connect, // otherwise the connection would fail. @@ -97,6 +110,9 @@ func (gm *Guest) CreateConnection(ctx context.Context, GCSServiceID guid.GUID, o // CloseConnection closes any active GCS connection and listener. func (gm *Guest) CloseConnection() error { + gm.mu.Lock() + defer gm.mu.Unlock() + var err error if gm.gc != nil { diff --git a/internal/vm/guestmanager/manager.go b/internal/vm/guestmanager/manager.go index 1db0b4da7a..2eeb705bbd 100644 --- a/internal/vm/guestmanager/manager.go +++ b/internal/vm/guestmanager/manager.go @@ -7,7 +7,6 @@ import ( "fmt" "github.com/Microsoft/hcsshim/internal/cmd" - "github.com/Microsoft/hcsshim/internal/cow" "github.com/Microsoft/hcsshim/internal/gcs" "github.com/Microsoft/go-winio/pkg/guid" @@ -26,10 +25,6 @@ type Manager interface { // Once the container is created, it can be managed using the returned `gcs.Container` interface. // `gcs.Container` uses the underlying guest connection to issue commands to the guest. CreateContainer(ctx context.Context, cid string, config interface{}) (*gcs.Container, error) - // CreateProcess creates a process in the guest. - // Once the process is created, it can be managed using the returned `cow.Process` interface. - // `cow.Process` uses the underlying guest connection to issue commands to the guest. - CreateProcess(ctx context.Context, settings interface{}) (cow.Process, error) // DumpStacks requests a stack dump from the guest and returns it as a string. DumpStacks(ctx context.Context) (string, error) // DeleteContainerState removes persisted state for the container identified by `cid` from the guest. @@ -42,11 +37,17 @@ var _ Manager = (*Guest)(nil) // Capabilities returns the capabilities of the guest connection. func (gm *Guest) Capabilities() gcs.GuestDefinedCapabilities { + gm.mu.RLock() + defer gm.mu.RUnlock() + return gm.gc.Capabilities() } // CreateContainer creates a container in the guest with the given ID and config. func (gm *Guest) CreateContainer(ctx context.Context, cid string, config interface{}) (*gcs.Container, error) { + gm.mu.Lock() + defer gm.mu.Unlock() + c, err := gm.gc.CreateContainer(ctx, cid, config) if err != nil { return nil, fmt.Errorf("failed to create container %s: %w", cid, err) @@ -55,18 +56,11 @@ func (gm *Guest) CreateContainer(ctx context.Context, cid string, config interfa return c, nil } -// CreateProcess creates a process in the guest using the provided settings. -func (gm *Guest) CreateProcess(ctx context.Context, settings interface{}) (cow.Process, error) { - p, err := gm.gc.CreateProcess(ctx, settings) - if err != nil { - return nil, fmt.Errorf("failed to create process: %w", err) - } - - return p, nil -} - // DumpStacks requests a stack dump from the guest and returns it as a string. func (gm *Guest) DumpStacks(ctx context.Context) (string, error) { + gm.mu.Lock() + defer gm.mu.Unlock() + dump, err := gm.gc.DumpStacks(ctx) if err != nil { return "", fmt.Errorf("failed to dump stacks: %w", err) @@ -77,6 +71,9 @@ func (gm *Guest) DumpStacks(ctx context.Context) (string, error) { // DeleteContainerState removes persisted state for the container identified by cid from the guest. func (gm *Guest) DeleteContainerState(ctx context.Context, cid string) error { + gm.mu.Lock() + defer gm.mu.Unlock() + err := gm.gc.DeleteContainerState(ctx, cid) if err != nil { return fmt.Errorf("failed to delete container state for container %s: %w", cid, err) diff --git a/internal/vm/guestmanager/request_helpers.go b/internal/vm/guestmanager/request_helpers.go index 1077d11fbd..60481abf5e 100644 --- a/internal/vm/guestmanager/request_helpers.go +++ b/internal/vm/guestmanager/request_helpers.go @@ -12,6 +12,9 @@ var errGuestConnectionUnavailable = errors.New("guest connection not initialized // modify sends a guest modification request via the guest connection. // This is a helper method to avoid having to check for a nil guest connection in every method that needs to send a request. func (gm *Guest) modify(ctx context.Context, req interface{}) error { + gm.mu.Lock() + defer gm.mu.Unlock() + if gm.gc == nil { return errGuestConnectionUnavailable }