[V2 Shims] Introduce locking at vmmanager and guestmanager layers

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 UVM 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 {

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)

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
 	}

internal/vm/vmmanager/lifetime.go

@@ -64,16 +64,25 @@ var _ LifetimeManager = (*UtilityVM)(nil)
 
 // ID returns the ID of the utility VM.
 func (uvm *UtilityVM) ID() string {
+	uvm.mu.RLock()
+	defer uvm.mu.RUnlock()
+
 	return uvm.id
 }
 
 // RuntimeID returns the runtime ID of the utility VM.
 func (uvm *UtilityVM) RuntimeID() guid.GUID {
+	uvm.mu.RLock()
+	defer uvm.mu.RUnlock()
+
 	return uvm.vmID
 }
 
 // Start starts the utility VM.
 func (uvm *UtilityVM) Start(ctx context.Context) (err error) {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	if err := uvm.cs.Start(ctx); err != nil {
 		return fmt.Errorf("failed to start utility VM: %w", err)
 	}
@@ -82,17 +91,23 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) {
 
 // Terminate terminates the utility VM and waits for it to exit.
 func (uvm *UtilityVM) Terminate(ctx context.Context) error {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	if err := uvm.cs.Terminate(ctx); err != nil {
 		return fmt.Errorf("failed to terminate utility VM: %w", err)
 	}
-	if err := uvm.Wait(ctx); err != nil {
+	if err := uvm.cs.WaitCtx(ctx); err != nil {
 		return fmt.Errorf("failed to wait for utility VM termination: %w", err)
 	}
 	return nil
 }
 
 // Close closes the utility VM and releases all associated resources.
 func (uvm *UtilityVM) Close(ctx context.Context) error {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	if err := uvm.cs.CloseCtx(ctx); err != nil {
 		return fmt.Errorf("failed to close utility VM: %w", err)
 	}
@@ -101,6 +116,9 @@ func (uvm *UtilityVM) Close(ctx context.Context) error {
 
 // Pause pauses the utility VM.
 func (uvm *UtilityVM) Pause(ctx context.Context) error {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	if err := uvm.cs.Pause(ctx); err != nil {
 		return fmt.Errorf("failed to pause utility VM: %w", err)
 	}
@@ -109,6 +127,9 @@ func (uvm *UtilityVM) Pause(ctx context.Context) error {
 
 // Resume resumes the utility VM.
 func (uvm *UtilityVM) Resume(ctx context.Context) error {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	if err := uvm.cs.Resume(ctx); err != nil {
 		return fmt.Errorf("failed to resume utility VM: %w", err)
 	}
@@ -117,6 +138,9 @@ func (uvm *UtilityVM) Resume(ctx context.Context) error {
 
 // Save saves the state of the utility VM as a template.
 func (uvm *UtilityVM) Save(ctx context.Context, options hcsschema.SaveOptions) error {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	if err := uvm.cs.Save(ctx, options); err != nil {
 		return fmt.Errorf("failed to save utility VM state: %w", err)
 	}
@@ -125,11 +149,17 @@ func (uvm *UtilityVM) Save(ctx context.Context, options hcsschema.SaveOptions) e
 
 // Wait waits for the utility VM to exit and returns any error that occurred during execution.
 func (uvm *UtilityVM) Wait(ctx context.Context) error {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	return uvm.cs.WaitCtx(ctx)
 }
 
 // PropertiesV2 returns the properties of the utility VM from HCS.
 func (uvm *UtilityVM) PropertiesV2(ctx context.Context, types ...hcsschema.PropertyType) (*hcsschema.Properties, error) {
+	uvm.mu.Lock()
+	defer uvm.mu.Unlock()
+
 	props, err := uvm.cs.PropertiesV2(ctx, types...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get properties from HCS: %w", err)
@@ -140,15 +170,24 @@ func (uvm *UtilityVM) PropertiesV2(ctx context.Context, types ...hcsschema.Prope
 
 // StartedTime returns the time when the utility VM entered the running state.
 func (uvm *UtilityVM) StartedTime() time.Time {
+	uvm.mu.RLock()
+	defer uvm.mu.RUnlock()
+
 	return uvm.cs.StartedTime()
 }
 
 // StoppedTime returns the time when the utility VM entered the stopped state.
 func (uvm *UtilityVM) StoppedTime() time.Time {
+	uvm.mu.RLock()
+	defer uvm.mu.RUnlock()
+
 	return uvm.cs.StoppedTime()
 }
 
 // ExitError returns the exit error of the utility VM, if it has exited.
 func (uvm *UtilityVM) ExitError() error {
+	uvm.mu.RLock()
+	defer uvm.mu.RUnlock()
+
 	return uvm.cs.ExitError()
 }

internal/vm/vmmanager/network.go

@@ -34,7 +34,7 @@ func (uvm *UtilityVM) AddNIC(ctx context.Context, nicID string, settings *hcssch
 		ResourcePath: fmt.Sprintf(resourcepaths.NetworkResourceFormat, nicID),
 		Settings:     settings,
 	}
-	if err := uvm.cs.Modify(ctx, request); err != nil {
+	if err := uvm.modify(ctx, request); err != nil {
 		return fmt.Errorf("failed to add NIC %s: %w", nicID, err)
 	}
 	return nil
@@ -46,7 +46,7 @@ func (uvm *UtilityVM) RemoveNIC(ctx context.Context, nicID string, settings *hcs
 		ResourcePath: fmt.Sprintf(resourcepaths.NetworkResourceFormat, nicID),
 		Settings:     settings,
 	}
-	if err := uvm.cs.Modify(ctx, request); err != nil {
+	if err := uvm.modify(ctx, request); err != nil {
 		return fmt.Errorf("failed to remove NIC %s: %w", nicID, err)
 	}
 	return nil
@@ -61,7 +61,7 @@ func (uvm *UtilityVM) UpdateNIC(ctx context.Context, nicID string, settings *hcs
 		ResourcePath: fmt.Sprintf(resourcepaths.NetworkResourceFormat, nicID),
 		Settings:     settings,
 	}
-	if err := uvm.cs.Modify(ctx, req); err != nil {
+	if err := uvm.modify(ctx, req); err != nil {
 		return fmt.Errorf("failed to update NIC %s: %w", nicID, err)
 	}
 	return nil

internal/vm/vmmanager/pci.go

@@ -28,7 +28,7 @@ func (uvm *UtilityVM) AddDevice(ctx context.Context, vmbusGUID string, settings
 		RequestType:  guestrequest.RequestTypeAdd,
 		Settings:     settings,
 	}
-	if err := uvm.cs.Modify(ctx, request); err != nil {
+	if err := uvm.modify(ctx, request); err != nil {
 		return fmt.Errorf("failed to add PCI device %s: %w", vmbusGUID, err)
 	}
 	return nil
@@ -39,7 +39,7 @@ func (uvm *UtilityVM) RemoveDevice(ctx context.Context, vmbusGUID string) error
 		ResourcePath: fmt.Sprintf(resourcepaths.VirtualPCIResourceFormat, vmbusGUID),
 		RequestType:  guestrequest.RequestTypeRemove,
 	}
-	if err := uvm.cs.Modify(ctx, request); err != nil {
+	if err := uvm.modify(ctx, request); err != nil {
 		return fmt.Errorf("failed to remove PCI device %s: %w", vmbusGUID, err)
 	}
 	return nil

internal/vm/vmmanager/pipe.go

@@ -27,7 +27,7 @@ func (uvm *UtilityVM) AddPipe(ctx context.Context, hostPath string) error {
 		RequestType:  guestrequest.RequestTypeAdd,
 		ResourcePath: fmt.Sprintf(resourcepaths.MappedPipeResourceFormat, hostPath),
 	}
-	if err := uvm.cs.Modify(ctx, modification); err != nil {
+	if err := uvm.modify(ctx, modification); err != nil {
 		return fmt.Errorf("failed to add pipe %s to uvm: %w", hostPath, err)
 	}
 
@@ -39,7 +39,7 @@ func (uvm *UtilityVM) RemovePipe(ctx context.Context, hostPath string) error {
 		RequestType:  guestrequest.RequestTypeRemove,
 		ResourcePath: fmt.Sprintf(resourcepaths.MappedPipeResourceFormat, hostPath),
 	}
-	if err := uvm.cs.Modify(ctx, modification); err != nil {
+	if err := uvm.modify(ctx, modification); err != nil {
 		return fmt.Errorf("failed to remove pipe %s from uvm: %w", hostPath, err)
 	}
 

internal/vm/vmmanager/plan9.go

@@ -28,7 +28,7 @@ func (uvm *UtilityVM) AddPlan9(ctx context.Context, settings hcsschema.Plan9Shar
 		Settings:     settings,
 		ResourcePath: resourcepaths.Plan9ShareResourcePath,
 	}
-	if err := uvm.cs.Modify(ctx, modification); err != nil {
+	if err := uvm.modify(ctx, modification); err != nil {
 		return fmt.Errorf("failed to add Plan9 share %s: %w", settings.Name, err)
 	}
 	return nil
@@ -40,7 +40,7 @@ func (uvm *UtilityVM) RemovePlan9(ctx context.Context, settings hcsschema.Plan9S
 		Settings:     settings,
 		ResourcePath: resourcepaths.Plan9ShareResourcePath,
 	}
-	if err := uvm.cs.Modify(ctx, modification); err != nil {
+	if err := uvm.modify(ctx, modification); err != nil {
 		return fmt.Errorf("failed to remove Plan9 share %s: %w", settings.Name, err)
 	}
 	return nil

internal/vm/vmmanager/resources.go

@@ -32,7 +32,7 @@ func (uvm *UtilityVM) SetCPUGroup(ctx context.Context, settings *hcsschema.CpuGr
 		ResourcePath: resourcepaths.CPUGroupResourcePath,
 		Settings:     settings,
 	}
-	if err := uvm.cs.Modify(ctx, modification); err != nil {
+	if err := uvm.modify(ctx, modification); err != nil {
 		return fmt.Errorf("failed to modify CPU Group: %w", err)
 	}
 	return nil
@@ -43,7 +43,7 @@ func (uvm *UtilityVM) UpdateCPULimits(ctx context.Context, settings *hcsschema.P
 		ResourcePath: resourcepaths.CPULimitsResourcePath,
 		Settings:     settings,
 	}
-	if err := uvm.cs.Modify(ctx, modification); err != nil {
+	if err := uvm.modify(ctx, modification); err != nil {
 		return fmt.Errorf("failed to modify CPU Limits: %w", err)
 	}
 	return nil
@@ -54,7 +54,7 @@ func (uvm *UtilityVM) UpdateMemory(ctx context.Context, memory uint64) error {
 		ResourcePath: resourcepaths.MemoryResourcePath,
 		Settings:     memory,
 	}
-	if err := uvm.cs.Modify(ctx, modification); err != nil {
+	if err := uvm.modify(ctx, modification); err != nil {
 		return fmt.Errorf("failed to modify memory: %w", err)
 	}
 	return nil

internal/vm/vmmanager/scsi.go

@@ -28,7 +28,7 @@ func (uvm *UtilityVM) AddSCSIDisk(ctx context.Context, disk hcsschema.Attachment
 		Settings:     disk,
 		ResourcePath: fmt.Sprintf(resourcepaths.SCSIResourceFormat, guestrequest.ScsiControllerGuids[controller], lun),
 	}
-	if err := uvm.cs.Modify(ctx, request); err != nil {
+	if err := uvm.modify(ctx, request); err != nil {
 		return fmt.Errorf("failed to add SCSI disk %s: %w", disk.Path, err)
 	}
 
@@ -41,7 +41,7 @@ func (uvm *UtilityVM) RemoveSCSIDisk(ctx context.Context, controller uint, lun u
 		ResourcePath: fmt.Sprintf(resourcepaths.SCSIResourceFormat, guestrequest.ScsiControllerGuids[controller], lun),
 	}
 
-	if err := uvm.cs.Modify(ctx, request); err != nil {
+	if err := uvm.modify(ctx, request); err != nil {
 		return fmt.Errorf("failed to remove SCSI disk %s: %w", request.ResourcePath, err)
 	}
 	return nil