From eb20df99fd39b3f7aca14a4d3d96b85eb2e26c03 Mon Sep 17 00:00:00 2001 From: Evan Dorsky Date: Thu, 11 Jun 2026 10:50:11 -0400 Subject: [PATCH] Fix tests and guard against panic --- logging/windows_etw_logger.go | 23 +++++++++-------------- logging/windows_logging_test.go | 5 +++-- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/logging/windows_etw_logger.go b/logging/windows_etw_logger.go index a8887ebd2c0..ee9db0e1119 100644 --- a/logging/windows_etw_logger.go +++ b/logging/windows_etw_logger.go @@ -72,7 +72,6 @@ func RegisterETWLogger(rootLogger Logger, etlDir string, p ETWProvider) (io.Clos if err := sess.Start(startCtx); err != nil { rootLogger.Warnw("ETW session start failed; provider registered but file capture could not start", "err", err, "session", p.SessionName, "outputPath", etlPath) - provider.Close() return nopCloser{}, err } else { liveSession = sess @@ -88,7 +87,7 @@ func RegisterETWLogger(rootLogger Logger, etlDir string, p ETWProvider) (io.Clos // listening, near-free when not — no buffering goroutine needed. type etwAppender struct { provider *etw.Provider - session sessionController // nil if session start failed + session sessionController } // Write maps the zap entry to a level-tagged ETW event with structured fields. @@ -142,20 +141,16 @@ func (a *etwAppender) Write(entry zapcore.Entry, fields []zapcore.Field) error { func (a *etwAppender) Sync() error { return nil } -// Close stops the session before unregistering the provider so any kernel -// buffer contents are flushed to the .etl file before teardown. +// stop the session if it still exists when Close() is called. +// don't touch the provider to avoid a bug in the Close logic of winio +// the provider is killed by windows when the process dies func (a *etwAppender) Close() error { - var sessErr error - if a.session != nil { - stopCtx, cancel := context.WithTimeout(context.Background(), etwLogmanTimeout) - defer cancel() - sessErr = a.session.Stop(stopCtx) - } - provErr := a.provider.Close() - if sessErr != nil { - return sessErr + if a.session == nil { + return nil } - return provErr + stopCtx, cancel := context.WithTimeout(context.Background(), etwLogmanTimeout) + defer cancel() + return a.session.Stop(stopCtx) } func zapToETWLevel(l zapcore.Level) etw.Level { diff --git a/logging/windows_logging_test.go b/logging/windows_logging_test.go index b8fd2ef4133..dc4c88b5918 100644 --- a/logging/windows_logging_test.go +++ b/logging/windows_logging_test.go @@ -26,6 +26,7 @@ func TestETWNulls(t *testing.T) { closer, err := RegisterETWLogger(logger, etlDir, ServerETW) test.That(t, closer, test.ShouldNotBeNil) test.That(t, err, test.ShouldBeNil) + defer closer.Close() logger.Info("message through registered ETW appender") err = logger.Sync() @@ -58,7 +59,7 @@ func TestETWAppenderLifecycle(t *testing.T) { test.That(t, a.Close(), test.ShouldBeNil) - // Write after close: provider is unregistered, pkg/etw's WriteEvent checks - // provider.enabled and no-ops. Must not panic. + // Write after close: there's no active session, so the provider is disabled + // Must not panic. logger.Info("after close") }