Merge pull request #612 from ChimeraCoder/add-hook-race

Fix data race for hooks

entry.go

@@ -94,7 +94,10 @@ func (entry Entry) log(level Level, msg string) {
 	entry.Level = level
 	entry.Message = msg
 
-	if err := entry.Logger.Hooks.Fire(level, &entry); err != nil {
+	entry.Logger.mu.Lock()
+	err := entry.Logger.Hooks.Fire(level, &entry)
+	entry.Logger.mu.Unlock()
+	if err != nil {
 		entry.Logger.mu.Lock()
 		fmt.Fprintf(os.Stderr, "Failed to fire hook: %v\n", err)
 		entry.Logger.mu.Unlock()

hook_test.go

@@ -1,6 +1,7 @@
 package logrus
 
 import (
+	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -120,3 +121,24 @@ func TestErrorHookShouldFireOnError(t *testing.T) {
 		assert.Equal(t, hook.Fired, true)
 	})
 }
+
+func TestAddHookRace(t *testing.T) {
+	var wg sync.WaitGroup
+	wg.Add(2)
+	hook := new(ErrorHook)
+	LogAndAssertJSON(t, func(log *Logger) {
+		go func() {
+			defer wg.Done()
+			log.AddHook(hook)
+		}()
+		go func() {
+			defer wg.Done()
+			log.Error("test")
+		}()
+		wg.Wait()
+	}, func(fields Fields) {
+		// the line may have been logged
+		// before the hook was added, so we can't
+		// actually assert on the hook
+	})
+}

logger.go

@@ -315,3 +315,9 @@ func (logger *Logger) level() Level {
 func (logger *Logger) SetLevel(level Level) {
 	atomic.StoreUint32((*uint32)(&logger.Level), uint32(level))
 }
+
+func (logger *Logger) AddHook(hook Hook) {
+	logger.mu.Lock()
+	defer logger.mu.Unlock()
+	logger.Hooks.Add(hook)
+}