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

[muwaqar-cflt]

Summary

loader.New() (internal/envoygateway/config/loader/configloader.go) stores the *config.Server pointer passed to it without copying it, so it's aliased with the cfg variable held by server() (internal/cmd/server.go). On a config-file reload, the loader's watcher goroutine reassigns cfg.Logger (and cfg.EnvoyGateway) under cfgMu. Meanwhile, server()'s own top-level select loop reads cfg.Logger directly (cfg.Logger.Error(...) / cfg.Logger.Info(...)) with no synchronization at all — it has no knowledge of cfgMu. That's an unsynchronized concurrent read/write on the same struct field: a genuine data race.

This was caught by go test -race in TestCustomProviderCancelWhenConfigReload:

WARNING: DATA RACE
Write at 0x00c000d0fba8 by goroutine 629:
  .../configloader.go:91 ...   (watcher goroutine's cfg.Logger reassignment)
Previous read at ...
  .../server.go:112 ...        (server()'s cfg.Logger.Info("shutting down"))

Reproduced locally under -race (needed Linux, since fsnotify's event-duplication behavior differs enough from macOS/kqueue that it didn't reproduce as reliably there).

Fix

The loader now publishes each new logger over a small "latest value" channel (LoggerUpdates()) as reloads happen. server() keeps its own local logger variable and swaps it only upon receiving from that channel in its select loop, so the update is synchronized via channel send/receive rather than a shared, unguarded struct field.

Test plan

• go build ./...
• go vet ./internal/cmd/... ./internal/envoygateway/config/loader/...
• go test -race -run TestCustomProviderCancelWhenConfigReload -count=100+ ./internal/cmd/... — reproduced the exact race pre-fix (Linux/Docker); 100/100 clean post-fix (both macOS and Linux)

[netlify]

✅ Deploy Preview for cerulean-figolla-1f9435 ready!

Name	Link
🔨 Latest commit	0814e1c
🔍 Latest deploy log	https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a67d4147e1d9b0007b8c05b
😎 Deploy Preview	https://deploy-preview-9581--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0ab8c14be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.67%. Comparing base (8782e63) to head (0814e1c).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9581      +/-   ##
==========================================
+ Coverage   75.64%   75.67%   +0.02%     
==========================================
  Files         252      254       +2     
  Lines       41758    41937     +179     
==========================================
+ Hits        31589    31735     +146     
- Misses       8046     8071      +25     
- Partials     2123     2131       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[zhaohuabing]

Nice find, but I think this is a bit overengineered. The loader already has cfgMu, so a read-locked getter does the job in ~7 lines with no channel, no drain logic, and no new exported API:

func (r *Loader) Logger() logging.Logger {
	r.cfgMu.RLock()
	defer r.cfgMu.RUnlock()
	return r.cfg.Logger
}

	for {
		select {
		// Exit if the config loader fails to start the runners.
		// Continuing with failed runners would cause EG to function incorrectly.
		case err := <-l.Errors():
			// 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():
			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()
			return nil
		}
	}

[muwaqar-cflt]

Nice find, but I think this is a bit overengineered. The loader already has cfgMu, so a read-locked getter does the job in ~7 lines with no channel, no drain logic, and no new exported API:

I was under the impression that undergoing a lock contention every time we need to access a logger here was not preferred, hence this version.

[zhaohuabing]

Nice find, but I think this is a bit overengineered. The loader already has cfgMu, so a read-locked getter does the job in ~7 lines with no channel, no drain logic, and no new exported API:

I was under the impression that undergoing a lock contention every time we need to access a logger here was not preferred, hence this version.

The lock is only taken inside the configloader, and Logger() would be called once per process lifetime, so there's effectively no overhead. I’d prefer the simpler fix here.

[muwaqar-cflt]

@zhaohuabing implemented the simpler version

[zhaohuabing]

LGTM. Thanks!

[jukie]

Thanks!

[jukie]

/retest