From be569094e9c87b6de4fa8909e1e7db1603702f9f Mon Sep 17 00:00:00 2001 From: Jay Ching Lim Date: Mon, 12 Feb 2018 17:26:48 -0500 Subject: [PATCH] Make fireHooks() method receive a copy of Entry structure to avoid race conditions Signed-off-by: Jay Ching Lim --- entry.go | 6 ++++-- hooks/test/test_test.go | 26 ++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/entry.go b/entry.go index df6f92d..778f4c9 100644 --- a/entry.go +++ b/entry.go @@ -113,10 +113,12 @@ func (entry Entry) log(level Level, msg string) { } } -func (entry *Entry) fireHooks() { +// This function is not declared with a pointer value because otherwise +// race conditions will occur when using multiple goroutines +func (entry Entry) fireHooks() { entry.Logger.mu.Lock() defer entry.Logger.mu.Unlock() - err := entry.Logger.Hooks.Fire(entry.Level, entry) + err := entry.Logger.Hooks.Fire(entry.Level, &entry) if err != nil { fmt.Fprintf(os.Stderr, "Failed to fire hook: %v\n", err) } diff --git a/hooks/test/test_test.go b/hooks/test/test_test.go index 3f55cfe..dea768e 100644 --- a/hooks/test/test_test.go +++ b/hooks/test/test_test.go @@ -1,6 +1,7 @@ package test import ( + "sync" "testing" "github.com/sirupsen/logrus" @@ -8,7 +9,6 @@ import ( ) func TestAllHooks(t *testing.T) { - assert := assert.New(t) logger, hook := NewNullLogger() @@ -35,5 +35,27 @@ func TestAllHooks(t *testing.T) { assert.Equal(logrus.ErrorLevel, hook.LastEntry().Level) assert.Equal("Hello error", hook.LastEntry().Message) assert.Equal(1, len(hook.Entries)) - +} + +func TestLoggingWithHooksRace(t *testing.T) { + assert := assert.New(t) + logger, hook := NewNullLogger() + + var wg sync.WaitGroup + wg.Add(100) + + for i := 0; i < 100; i++ { + go func() { + logger.Info("info") + wg.Done() + }() + } + + assert.Equal(logrus.InfoLevel, hook.LastEntry().Level) + assert.Equal("info", hook.LastEntry().Message) + + wg.Wait() + + entries := hook.AllEntries() + assert.Equal(100, len(entries)) }