fix: eliminate cfg.Logger data race in server()'s reload/shutdown path (#9581)

* fix: eliminate cfg.Logger data race in server()'s reload/shutdown path

Loader.New() aliases the *config.Server passed to it, so the watcher
goroutine's cfg.Logger reassignment on each config reload directly
mutates the same struct instance server()'s top-level select loop
reads from when logging shutdown/hook-error messages. The watcher
writes under cfgMu, but server() read cfg.Logger with no
synchronization at all, producing a genuine data race (confirmed via
the actual CI job log: write at configloader.go:91 vs. read at
server.go:112, and reproduced locally under linux/-race).

Fix: the loader publishes each new logger over a small "latest value"
channel as reloads happen. server() keeps its own local logger
reference and swaps it only upon receiving from that channel, so the
update is synchronized via channel send/receive instead of a shared,
unguarded field.

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

* 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>

---------

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

Author: muwaqar-cflt

internal/cmd/server.go

@@ -102,14 +102,17 @@ func server(ctx context.Context, stdout, stderr io.Writer, cfgPath string, hook
 		// Exit if the config loader fails to start the runners.
 		// Continuing with failed runners would cause EG to function incorrectly.
 		case err := <-l.Errors():
-			cfg.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():
-			cfg.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

@@ -142,6 +142,14 @@ func (r *Loader) Errors() <-chan error {
 	return r.hookErr
 }
 
+// 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.
 func (r *Loader) Wait() {
 	r.hookMutex.Lock()

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

@@ -0,0 +1 @@
+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.