introduce lock in guestmanager to address concurrency at bottom layer

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>

Author: rawahars

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,13 @@ 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
+
+	// gcsServiceID is the GUID that the guest uses to connect to GCS.
+	gcsServiceID guid.GUID
+
 	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 +63,20 @@ 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 {
+		// If the caller tried to connect to a different GCS service then error out.
+		if gm.gcsServiceID != GCSServiceID {
+			return fmt.Errorf("gcs service id mismatch: expected %s, got %s", gm.gcsServiceID, GCSServiceID)
+		}
+		return nil
+	}
+
+	gm.gcsServiceID = GCSServiceID
+
 	// 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 +119,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
 	}