From a774f92bf7410afc2a0c3fb4a5b4bba91cb11761 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ask=20Bj=C3=B8rn=20Hansen?= Date: Sat, 2 Aug 2025 22:55:57 -0700 Subject: [PATCH] fix(logger): prevent mutex crash in bufferingExporter Remove sync.Once reset that caused "unlock of unlocked mutex" panic. Redesign initialization to use only checkReadiness goroutine for retry attempts, eliminating race condition while preserving retry functionality for TLS/tracing setup delays. --- logger/buffering_exporter.go | 62 ++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/logger/buffering_exporter.go b/logger/buffering_exporter.go index 6d7bdb1..3774f54 100644 --- a/logger/buffering_exporter.go +++ b/logger/buffering_exporter.go @@ -23,9 +23,8 @@ type bufferingExporter struct { // Real exporter (created when tracing is configured) exporter otellog.Exporter - // Thread-safe initialization - initOnce sync.Once - initErr error + // Thread-safe initialization state (managed only by checkReadiness) + initErr error // Background checker stopChecker chan struct{} @@ -48,20 +47,13 @@ func newBufferingExporter() *bufferingExporter { // Export implements otellog.Exporter func (e *bufferingExporter) Export(ctx context.Context, records []otellog.Record) error { - // Try initialization once - e.initOnce.Do(func() { - e.initErr = e.initialize() - }) + // Check if exporter is ready (initialization handled by checkReadiness goroutine) + e.mu.RLock() + exporter := e.exporter + e.mu.RUnlock() - // If initialization succeeded, use the exporter - if e.initErr == nil { - e.mu.RLock() - exporter := e.exporter - e.mu.RUnlock() - - if exporter != nil { - return exporter.Export(ctx, records) - } + if exporter != nil { + return exporter.Export(ctx, records) } // Not ready yet, buffer the records @@ -117,24 +109,31 @@ func (e *bufferingExporter) bufferRecords(records []otellog.Record) error { return nil } -// checkReadiness periodically checks if tracing is configured +// checkReadiness periodically attempts initialization until successful func (e *bufferingExporter) checkReadiness() { defer close(e.checkerDone) - ticker := time.NewTicker(1 * time.Second) // Reduced frequency since OTLP handles retries + ticker := time.NewTicker(1 * time.Second) defer ticker.Stop() for { select { case <-ticker.C: - // If initialization failed, reset sync.Once to allow retry - // The OTLP exporter will handle its own retry logic - if e.initErr != nil { - e.initOnce = sync.Once{} - } else if e.exporter != nil { + // Check if we already have a working exporter + e.mu.RLock() + hasExporter := e.exporter != nil + e.mu.RUnlock() + + if hasExporter { return // Exporter ready, checker no longer needed } + // Try to initialize + err := e.initialize() + e.mu.Lock() + e.initErr = err + e.mu.Unlock() + case <-e.stopChecker: return } @@ -180,14 +179,21 @@ func (e *bufferingExporter) Shutdown(ctx context.Context) error { // Stop the readiness checker from continuing close(e.stopChecker) - // Give one final chance for TLS/tracing to become ready before fully shutting down - e.initOnce.Do(func() { - e.initErr = e.initialize() - }) - // Wait for readiness checker goroutine to complete <-e.checkerDone + // Give one final chance for TLS/tracing to become ready for buffer flushing + e.mu.RLock() + hasExporter := e.exporter != nil + e.mu.RUnlock() + + if !hasExporter { + err := e.initialize() + e.mu.Lock() + e.initErr = err + e.mu.Unlock() + } + e.mu.Lock() defer e.mu.Unlock()