From c138d573a3568151502a39ba9257cf657dcc8a4c Mon Sep 17 00:00:00 2001 From: Jason Barron Date: Wed, 7 Nov 2018 09:17:02 +0100 Subject: [PATCH] Fix panic due to missing mutex lock/unlock We cannot just re-create the 'tempLabels' struct here without first checking if the mutex is locked. The simplest solution is to have a reset() function which resets the internal map, but preserves the mutex state. --- core.go | 2 +- core_test.go | 28 ++++++++++++++++++++++++++++ label.go | 6 ++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/core.go b/core.go index 886d665..5684fe4 100644 --- a/core.go +++ b/core.go @@ -82,7 +82,7 @@ func (c *core) Write(ent zapcore.Entry, fields []zapcore.Field) error { fields = append(fields, labelsField(c.allLabels())) fields = c.withSourceLocation(ent, fields) - c.tempLabels = newLabels() + c.tempLabels.reset() return c.Core.Write(ent, fields) } diff --git a/core_test.go b/core_test.go index cef1233..d640c90 100644 --- a/core_test.go +++ b/core_test.go @@ -2,6 +2,7 @@ package zapdriver import ( "runtime" + "sync/atomic" "testing" "github.com/stretchr/testify/require" @@ -110,6 +111,33 @@ func TestWrite(t *testing.T) { assert.NotNil(t, logs.All()[0].ContextMap()["labels"]) } +func TestWriteConcurrent(t *testing.T) { + temp := newLabels() + temp.store = map[string]string{"one": "1", "two": "2"} + goRoutines := 8 + counter := int32(10000) + + debugcore, logs := observer.New(zapcore.DebugLevel) + core := &core{debugcore, newLabels(), temp} + + fields := []zap.Field{ + zap.String("hello", "world"), + Label("one", "value"), + Label("two", "value"), + } + + for i := 0; i < goRoutines; i++ { + go func() { + for atomic.AddInt32(&counter, -1) > 0 { + err := core.Write(zapcore.Entry{}, fields) + require.NoError(t, err) + } + }() + } + + assert.NotNil(t, logs.All()[0].ContextMap()["labels"]) +} + func TestWithAndWrite(t *testing.T) { debugcore, logs := observer.New(zapcore.DebugLevel) core := zapcore.Core(&core{debugcore, newLabels(), newLabels()}) diff --git a/label.go b/label.go index 0a2d149..07656c9 100644 --- a/label.go +++ b/label.go @@ -58,6 +58,12 @@ func (l *labels) Add(key, value string) { l.mutex.Unlock() } +func (l *labels) reset() { + l.mutex.Lock() + l.store = map[string]string{} + l.mutex.Unlock() +} + func (l labels) MarshalLogObject(enc zapcore.ObjectEncoder) error { l.mutex.RLock() for k, v := range l.store {