From fb71758054e7cd2eace15c75474540e93277d3db Mon Sep 17 00:00:00 2001 From: Koichi Shiraishi Date: Fri, 25 Jun 2021 03:53:20 +0900 Subject: [PATCH] zapcore: add stopped for Stop method called exactly once (#966) --- zapcore/buffered_write_syncer.go | 20 ++++++++++++++++---- zapcore/buffered_write_syncer_test.go | 8 ++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/zapcore/buffered_write_syncer.go b/zapcore/buffered_write_syncer.go index f08ca7e81..0c1436f76 100644 --- a/zapcore/buffered_write_syncer.go +++ b/zapcore/buffered_write_syncer.go @@ -74,6 +74,7 @@ type BufferedWriteSyncer struct { writer *bufio.Writer ticker *time.Ticker stop chan struct{} // closed when flushLoop should stop + stopped bool // whether Stop() has run done chan struct{} // closed when flushLoop has stopped } @@ -155,9 +156,9 @@ func (s *BufferedWriteSyncer) flushLoop() { // Stop closes the buffer, cleans up background goroutines, and flushes // remaining unwritten data. -// -// This must be called exactly once. -func (s *BufferedWriteSyncer) Stop() error { +func (s *BufferedWriteSyncer) Stop() (err error) { + var stopped bool + // Critical section. func() { s.mu.Lock() @@ -167,10 +168,21 @@ func (s *BufferedWriteSyncer) Stop() error { return } + stopped = s.stopped + if stopped { + return + } + s.stopped = true + s.ticker.Stop() close(s.stop) // tell flushLoop to stop <-s.done // and wait until it has }() - return s.Sync() + // Don't call Sync on consecutive Stops. + if !stopped { + err = s.Sync() + } + + return err } diff --git a/zapcore/buffered_write_syncer_test.go b/zapcore/buffered_write_syncer_test.go index 9c28076bb..72d4d6f88 100644 --- a/zapcore/buffered_write_syncer_test.go +++ b/zapcore/buffered_write_syncer_test.go @@ -53,6 +53,14 @@ func TestBufferWriter(t *testing.T) { assert.Equal(t, "foo", buf.String(), "Unexpected log string") }) + t.Run("stop twice", func(t *testing.T) { + ws := &BufferedWriteSyncer{WS: &ztest.FailWriter{}} + _, err := ws.Write([]byte("foo")) + require.NoError(t, err, "Unexpected error writing to WriteSyncer.") + assert.Error(t, ws.Stop(), "Expected stop to fail.") + assert.NoError(t, ws.Stop(), "Expected stop to not fail.") + }) + t.Run("wrap twice", func(t *testing.T) { buf := &bytes.Buffer{} bufsync := &BufferedWriteSyncer{WS: AddSync(buf)}