Deadlock when calling BufferedWriteSyncer.Stop

[Gordy96]

Describe the bug

Upon calling Stop there's a small chance of deadlock. Happens sometimes on go v1.21.3 zap v1.22.0.

To Reproduce

Can't really figure out how to reproduce it without touching said code yet.

This is what I suspect is happening:

When Stop is called, it spawns a goroutine that locks mutex,

zap/zapcore/buffered_write_syncer.go

Lines 195 to 196 in b15585b

	s.mu.Lock()
	defer s.mu.Unlock()

stops timer and tells flushLoop to break

zap/zapcore/buffered_write_syncer.go

Lines 208 to 210 in b15585b

	s.ticker.Stop()
	close(s.stop) // tell flushLoop to stop
	<-s.done // and wait until it has

However if timer fired before stop occurred here

zap/zapcore/buffered_write_syncer.go

Lines 176 to 184 in b15585b

	select {
	case <-s.ticker.C:
	// we just simply ignore error here
	// because the underlying bufio writer stores any errors
	// and we return any error from Sync() as part of the close
	_ = s.Sync()
	case <-s.stop:
	return
	}

It calls Sync that tries to acuire locked mutex

zap/zapcore/buffered_write_syncer.go

Lines 159 to 160 in b15585b

	s.mu.Lock()
	defer s.mu.Unlock()

So Stop goroutine holds mutex and waits for flushLoop to close channel on defer while flushLoop waits on Sync to finish, but it can't since mutex is locked in Stop goroutine

[prashantv]

Thank you for the detailed bug report, you're exactly right with your analysis, Stop holds the lock, which flushLoop will also try to acquire if the timer ticks around the same time. I was able to reproduce it consistently with the added test in #1430, and verified that it fixes the issue.