fix: simplify logger race fix to a mutex-guarded Loader.Logger() accessor

Per review feedback (github.com//pull/9581#issuecomment-5077942710):
drop the loggerUpdates channel and drain logic in favor of a simple
RLock-guarded Logger() getter on Loader, reusing the existing cfgMu
rather than introducing new synchronization.

Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>

Author: muwaqar-cflt

internal/cmd/server.go

@@ -88,10 +88,6 @@ func server(ctx context.Context, stdout, stderr io.Writer, cfgPath string, hook
 		return err
 	}
 
-	// cfg.Logger is mutated concurrently on reload, so keep our own reference
-	// and update it only via l.LoggerUpdates() below, never read cfg.Logger again.
-	logger := cfg.Logger
-
 	l := loader.New(cfgPath, cfg, hook)
 	if err := l.Start(ctx, stdout); err != nil {
 		return err
@@ -103,20 +99,20 @@ func server(ctx context.Context, stdout, stderr io.Writer, cfgPath string, hook
 
 	for {
 		select {
-		// Pick up the latest logger whenever a config reload replaces it.
-		case newLogger := <-l.LoggerUpdates():
-			logger = newLogger
 		// Exit if the config loader fails to start the runners.
 		// Continuing with failed runners would cause EG to function incorrectly.
 		case err := <-l.Errors():
-			logger.Error(err, "failed to start runners")
+			// Read the logger through the loader so the read is guarded by the
+			// same mutex the reload path holds while replacing it. Reading
+			// cfg.Logger directly here races with that swap.
+			l.Logger().Error(err, "failed to start runners")
 			// Wait for runners to finish before shutting down.
 			// This is to make sure no orphaned runner process is left running in standalone mode.
 			l.Wait()
 			return err
 		// Wait for the context to be done, which usually happens the process receives a SIGTERM or SIGINT.
 		case <-ctx.Done():
-			logger.Info("shutting down")
+			l.Logger().Info("shutting down")
 			// Wait for runners to finish before shutting down.
 			// This is to make sure no orphaned runner process is left running in standalone mode.
 			l.Wait()

internal/envoygateway/config/loader/configloader.go

@@ -29,21 +29,17 @@ type Loader struct {
 	hook      HookFunc
 	hookErr   chan error
 
-	// loggerUpdates publishes the logger on each config reload; see LoggerUpdates.
-	loggerUpdates chan logging.Logger
-
 	w filewatcher.FileWatcher
 }
 
 func New(cfgPath string, cfg *config.Server, f HookFunc) *Loader {
 	return &Loader{
-		cfgPath:       cfgPath,
-		cfg:           cfg,
-		logger:        cfg.Logger.WithName("config-loader"),
-		hook:          f,
-		hookErr:       make(chan error, 1),
-		loggerUpdates: make(chan logging.Logger, 1),
-		w:             filewatcher.NewWatcher(),
+		cfgPath: cfgPath,
+		cfg:     cfg,
+		logger:  cfg.Logger.WithName("config-loader"),
+		hook:    f,
+		hookErr: make(chan error, 1),
+		w:       filewatcher.NewWatcher(),
 	}
 }
 
@@ -89,13 +85,11 @@ func (r *Loader) Start(ctx context.Context, logOut io.Writer) error {
 					continue
 				}
 
-				newLogger := logging.NewLogger(logOut, eg.Logging)
 				r.cfgMu.Lock()
 				r.cfg.EnvoyGateway = eg
 				// update cfg logger
-				r.cfg.Logger = newLogger
+				r.cfg.Logger = logging.NewLogger(logOut, eg.Logging)
 				r.cfgMu.Unlock()
-				r.publishLogger(newLogger)
 
 				// cancel last
 				if r.cancel != nil {
@@ -148,20 +142,12 @@ func (r *Loader) Errors() <-chan error {
 	return r.hookErr
 }
 
-// LoggerUpdates returns a channel that receives the latest logger on each config reload.
-func (r *Loader) LoggerUpdates() <-chan logging.Logger {
-	return r.loggerUpdates
-}
-
-func (r *Loader) publishLogger(l logging.Logger) {
-	select {
-	case <-r.loggerUpdates: // drain a stale, unconsumed value, if any
-	default:
-	}
-	select {
-	case r.loggerUpdates <- l:
-	default:
-	}
+// Logger returns the current logger, safe to call concurrently with a config
+// reload replacing it.
+func (r *Loader) Logger() logging.Logger {
+	r.cfgMu.RLock()
+	defer r.cfgMu.RUnlock()
+	return r.cfg.Logger
 }
 
 // Wait returns when success to acquire mutex, which means no hook is running.

release-notes/current/bug_fixes/9581-config-reload-server-logger-data-race.md

@@ -1 +1 @@
-Fixed a data race between config-reload and the standalone server's shutdown/error logging by having the config loader publish logger updates over a channel instead of a shared, unguarded field.
+Fixed a data race between config-reload and the standalone server's shutdown/error logging by reading the logger through a new mutex-guarded `Loader.Logger()` accessor instead of the shared `cfg.Logger` field directly.