Fix: remove close(l.buffer) to prevent send-on-closed-channel panic i… (#92)

SantiagoDePolonia · claude · web-flow · commit cf5e6ffdc6ee · 2026-02-25T16:33:15.000+01:00
* Fix: remove close(l.buffer) to prevent send-on-closed-channel panic in async loggers

A race existed between Write() and flushLoop() during shutdown: a goroutine
could pass the l.closed check, get preempted, and resume after close(l.buffer),
causing a panic. Closing the buffer channel is unnecessary since flushLoop exits
via l.done; the GC reclaims the channel. Drain now uses a non-blocking select
loop instead of range.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;

* Fix: increase SQLite connection pool to unlock WAL concurrent reads

The single-connection pool (MaxOpenConns=1) serialized all DB access,
negating WAL mode's native support for concurrent readers. Dashboard
read queries blocked on background flush writes and vice versa.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;

* Fix: add writes WaitGroup + idempotent Close() to audit logger

Mirrors the synchronization pattern already in usage/logger.go:
- writes WaitGroup tracks in-flight Write() calls so Close() waits
  for them before signaling flushLoop to stop
- Double-check pattern in Write() closes the race window between
  the first closed.Load() and writes.Add(1)
- Close() uses closed.Swap(true) for idempotency — safe to call
  multiple times during shutdown

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;

* Revert date-range helper inlining in usage readers

Restore sqliteDateRangeConditions(), pgDateRangeConditions(), and
mongoDateRangeFilter() helper functions. The inlining was an unrelated
refactoring regression that duplicated identical date-range logic across
12 call sites with no behavioral change.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;

---------

Co-authored-by: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/internal/auditlog/logger.go b/internal/auditlog/logger.go
@@ -17,6 +17,7 @@ type Logger struct {
 	buffer        chan *LogEntry
 	done          chan struct{}
 	wg            sync.WaitGroup
+	writes        sync.WaitGroup // tracks in-flight Write calls
 	flushInterval time.Duration
 	closed        atomic.Bool
 }
@@ -58,6 +59,15 @@ func (l *Logger) Write(entry *LogEntry) {
 		return
 	}
 
+	// Track this write to prevent Close from closing buffer while we're sending
+	l.writes.Add(1)
+	defer l.writes.Done()
+
+	// Double-check after registering - Close() may have set closed between first check and Add(1)
+	if l.closed.Load() {
+		return
+	}
+
 	select {
 	case l.buffer <- entry:
 		// Entry queued successfully
@@ -81,7 +91,16 @@ func (l *Logger) Config() Config {
 
 // Close stops the logger and flushes remaining entries.
 // This should be called during graceful shutdown.
+// Close is idempotent - calling it multiple times is safe.
 func (l *Logger) Close() error {
+	// Make Close idempotent - if already closed, return immediately
+	if l.closed.Swap(true) {
+		return nil
+	}
+
+	// Wait for any in-flight Write calls to complete
+	l.writes.Wait()
+
 	// Signal the flush loop to stop
 	close(l.done)
 
@@ -119,13 +138,19 @@ func (l *Logger) flushLoop() {
 			}
 
 		case <-l.done:
-			// Shutdown: mark as closed before closing buffer to prevent Write() panics
-			l.closed.Store(true)
-			close(l.buffer)
-			// Drain remaining entries from buffer
-			for entry := range l.buffer {
-				batch = append(batch, entry)
+			// Shutdown: drain remaining entries from buffer using non-blocking loop.
+			// Note: l.closed is already set by Close() before sending on l.done.
+			// We do NOT close(l.buffer) — closing is unnecessary since flushLoop
+			// exits via l.done, and closing creates a race with concurrent Write() calls.
+			for {
+				select {
+				case entry := <-l.buffer:
+					batch = append(batch, entry)
+				default:
+					goto drainComplete
+				}
 			}
+		drainComplete:
 			// Final flush
 			if len(batch) > 0 {
 				l.flushBatch(batch)
diff --git a/internal/storage/sqlite.go b/internal/storage/sqlite.go
@@ -34,10 +34,10 @@ func NewSQLite(cfg SQLiteConfig) (Storage, error) {
 		return nil, fmt.Errorf("failed to open SQLite database: %w", err)
 	}
 
-	// Use a single connection to serialize all database access.
-	// This prevents "database is locked" errors at the cost of no concurrent reads.
-	db.SetMaxOpenConns(1)
-	db.SetMaxIdleConns(1)
+	// Allow multiple readers to operate concurrently. WAL mode handles this natively.
+	// We keep a modest pool to prevent file descriptor exhaustion.
+	db.SetMaxOpenConns(16)
+	db.SetMaxIdleConns(4)
 
 	// Verify connection
 	if err := db.Ping(); err != nil {
diff --git a/internal/usage/logger.go b/internal/usage/logger.go
@@ -138,13 +138,19 @@ func (l *Logger) flushLoop() {
 			}
 
 		case <-l.done:
-			// Shutdown: close buffer to stop accepting entries
-			// Note: l.closed is already set by Close() before sending on l.done
-			close(l.buffer)
-			// Drain remaining entries from buffer
-			for entry := range l.buffer {
-				batch = append(batch, entry)
+			// Shutdown: drain remaining entries from buffer using non-blocking loop.
+			// Note: l.closed is already set by Close() before sending on l.done.
+			// We do NOT close(l.buffer) — closing is unnecessary since flushLoop
+			// exits via l.done, and closing creates a race with concurrent Write() calls.
+			for {
+				select {
+				case entry := <-l.buffer:
+					batch = append(batch, entry)
+				default:
+					goto drainComplete
+				}
 			}
+		drainComplete:
 			// Final flush
 			if len(batch) > 0 {
 				l.flushBatch(batch)
