use syncmap

Signed-off-by: Adrian Cole <[email protected]>

Author: codefromthecrypt

internal/infrastructure/host/infra.go

@@ -46,9 +46,7 @@ type Infra struct {
 	EnvoyGateway *egv1a1.EnvoyGateway
 
 	// proxyContextMap store the context of each running proxy by its name for lifecycle management.
-	proxyContextMap map[string]*proxyContext
-	// mu protects proxyContextMap from concurrent access.
-	mu sync.RWMutex
+	proxyContextMap sync.Map
 
 	// TODO: remove this field once it supports the configurable homeDir
 	sdsConfigPath string
@@ -82,7 +80,6 @@ func NewInfra(runnerCtx context.Context, cfg *config.Server, logger logging.Logg
 		HomeDir:           defaultHomeDir,
 		Logger:            logger,
 		EnvoyGateway:      cfg.EnvoyGateway,
-		proxyContextMap:   make(map[string]*proxyContext),
 		sdsConfigPath:     defaultLocalCertPathDir,
 		defaultEnvoyImage: egv1a1.DefaultEnvoyProxyImage,
 		Stdout:            cfg.Stdout,

internal/infrastructure/host/proxy_infra.go

@@ -34,12 +34,12 @@ type proxyContext struct {
 
 // Close implements the Manager interface.
 func (i *Infra) Close() error {
-	i.mu.RLock()
-	names := make([]string, 0, len(i.proxyContextMap))
-	for name := range i.proxyContextMap {
-		names = append(names, name)
-	}
-	i.mu.RUnlock()
+	// Collect all proxy names first to avoid holding locks during stopEnvoy calls
+	var names []string
+	i.proxyContextMap.Range(func(key, value interface{}) bool {
+		names = append(names, key.(string))
+		return true
+	})
 
 	for _, name := range names {
 		i.stopEnvoy(name)
@@ -60,10 +60,7 @@ func (i *Infra) CreateOrUpdateProxyInfra(ctx context.Context, infra *ir.Infra) e
 	proxyInfra := infra.GetProxyInfra()
 	proxyName := utils.GetHashedName(proxyInfra.Name, 64)
 	// Return directly if the proxy is running.
-	i.mu.RLock()
-	_, running := i.proxyContextMap[proxyName]
-	i.mu.RUnlock()
-	if running {
+	if _, loaded := i.proxyContextMap.Load(proxyName); loaded {
 		return nil
 	}
 
@@ -99,9 +96,7 @@ func (i *Infra) CreateOrUpdateProxyInfra(ctx context.Context, infra *ir.Infra) e
 func (i *Infra) runEnvoy(ctx context.Context, envoyVersion, name string, args []string) {
 	pCtx, cancel := context.WithCancel(ctx)
 	exit := make(chan struct{}, 1)
-	i.mu.Lock()
-	i.proxyContextMap[name] = &proxyContext{cancel: cancel, exit: exit}
-	i.mu.Unlock()
+	i.proxyContextMap.Store(name, &proxyContext{cancel: cancel, exit: exit})
 	go func() {
 		// Run blocks until pCtx is done or the process exits where the latter doesn't happen when
 		// Envoy successfully starts up. So, this will not return until pCtx is done in practice.
@@ -134,18 +129,12 @@ func (i *Infra) DeleteProxyInfra(_ context.Context, infra *ir.Infra) error {
 
 // stopEnvoy stops the Envoy process by its name. It will block until the process completely stopped.
 func (i *Infra) stopEnvoy(proxyName string) {
-	i.mu.Lock()
-	pCtx, ok := i.proxyContextMap[proxyName]
-	i.mu.Unlock()
-
+	value, ok := i.proxyContextMap.LoadAndDelete(proxyName)
 	if ok {
+		pCtx := value.(*proxyContext)
 		pCtx.cancel()    // Cancel causes the Envoy process to exit.
 		<-pCtx.exit      // Wait for the Envoy process to completely exit.
 		close(pCtx.exit) // Close the channel to avoid leaking.
-
-		i.mu.Lock()
-		delete(i.proxyContextMap, proxyName)
-		i.mu.Unlock()
 	}
 }
 

internal/infrastructure/host/proxy_infra_test.go

@@ -45,13 +45,12 @@ func newMockInfra(t *testing.T, cfg *config.Server) *Infra {
 	require.NoError(t, err)
 
 	infra := &Infra{
-		HomeDir:         homeDir,
-		Logger:          logging.DefaultLogger(io.Discard, egv1a1.LogLevelInfo),
-		EnvoyGateway:    cfg.EnvoyGateway,
-		proxyContextMap: make(map[string]*proxyContext),
-		sdsConfigPath:   proxyDir,
-		Stdout:          io.Discard,
-		Stderr:          io.Discard,
+		HomeDir:       homeDir,
+		Logger:        logging.DefaultLogger(io.Discard, egv1a1.LogLevelInfo),
+		EnvoyGateway:  cfg.EnvoyGateway,
+		sdsConfigPath: proxyDir,
+		Stdout:        io.Discard,
+		Stderr:        io.Discard,
 	}
 	return infra
 }
@@ -102,11 +101,10 @@ func TestInfra_runEnvoy_stopEnvoy(t *testing.T) {
 	stdout := &bytes.Buffer{}
 	stderr := &bytes.Buffer{}
 	i := &Infra{
-		proxyContextMap: make(map[string]*proxyContext),
-		HomeDir:         tmpdir,
-		Logger:          logging.DefaultLogger(stdout, egv1a1.LogLevelInfo),
-		Stdout:          stdout,
-		Stderr:          stderr,
+		HomeDir: tmpdir,
+		Logger:  logging.DefaultLogger(stdout, egv1a1.LogLevelInfo),
+		Stdout:  stdout,
+		Stderr:  stderr,
 	}
 	// Ensures that run -> stop will successfully stop the envoy and we can
 	// run it again without any issues.
@@ -116,9 +114,11 @@ func TestInfra_runEnvoy_stopEnvoy(t *testing.T) {
 			"admin: {address: {socket_address: {address: '127.0.0.1', port_value: 9901}}}",
 		}
 		i.runEnvoy(t.Context(), "", "test", args)
-		require.Len(t, i.proxyContextMap, 1)
+		_, ok := i.proxyContextMap.Load("test")
+		require.True(t, ok, "expected proxy context to be stored")
 		i.stopEnvoy("test")
-		require.Empty(t, i.proxyContextMap)
+		_, ok = i.proxyContextMap.Load("test")
+		require.False(t, ok, "expected proxy context to be removed")
 		// If the cleanup didn't work, the error due to "address already in use" will be tried to be written to the nil logger,
 		// which will panic.
 	}
@@ -165,11 +165,10 @@ func TestInfra_runEnvoy_OutputRedirection(t *testing.T) {
 	stderr := buffers[1]
 
 	i := &Infra{
-		proxyContextMap: make(map[string]*proxyContext),
-		HomeDir:         tmpdir,
-		Logger:          logging.DefaultLogger(stdout, egv1a1.LogLevelInfo),
-		Stdout:          stdout,
-		Stderr:          stderr,
+		HomeDir: tmpdir,
+		Logger:  logging.DefaultLogger(stdout, egv1a1.LogLevelInfo),
+		Stdout:  stdout,
+		Stderr:  stderr,
 	}
 
 	// Run envoy with an invalid config to force it to write to stderr and exit quickly
@@ -179,15 +178,17 @@ func TestInfra_runEnvoy_OutputRedirection(t *testing.T) {
 	}
 
 	i.runEnvoy(t.Context(), "", "test", args)
-	require.Len(t, i.proxyContextMap, 1)
+	_, ok := i.proxyContextMap.Load("test")
+	require.True(t, ok, "expected proxy context to be stored")
 
 	// Wait a bit for envoy to fail
 	require.Eventually(t, func() bool {
 		return stderr.Len() > 0 || stdout.Len() > 0
 	}, 5*time.Second, 100*time.Millisecond, "expected output to be written to buffers")
 
 	i.stopEnvoy("test")
-	require.Empty(t, i.proxyContextMap)
+	_, ok = i.proxyContextMap.Load("test")
+	require.False(t, ok, "expected proxy context to be removed")
 
 	// Verify that output was captured in buffers (either stdout or stderr should have content)
 	totalOutput := stdout.Len() + stderr.Len()