Merge pull request #527 from flimzy/race

Protect test hook entries with a mutex

hooks/test/test.go

@@ -2,16 +2,21 @@ package test
 
 import (
 	"io/ioutil"
+	"sync"
 
 	"github.com/Sirupsen/logrus"
 )
 
-// test.Hook is a hook designed for dealing with logs in test scenarios.
+// Hook is a hook designed for dealing with logs in test scenarios.
 type Hook struct {
+	// Entries is an array of all entries that have been received by this hook.
+	// For safe access, use the AllEntries() method, rather than reading this
+	// value directly.
 	Entries []*logrus.Entry
+	mu      sync.RWMutex
 }
 
-// Installs a test hook for the global logger.
+// NewGlobal installs a test hook for the global logger.
 func NewGlobal() *Hook {
 
 	hook := new(Hook)
@@ -21,7 +26,7 @@ func NewGlobal() *Hook {
 
 }
 
-// Installs a test hook for a given local logger.
+// NewLocal installs a test hook for a given local logger.
 func NewLocal(logger *logrus.Logger) *Hook {
 
 	hook := new(Hook)
@@ -31,7 +36,7 @@ func NewLocal(logger *logrus.Logger) *Hook {
 
 }
 
-// Creates a discarding logger and installs the test hook.
+// NewNullLogger creates a discarding logger and installs the test hook.
 func NewNullLogger() (*logrus.Logger, *Hook) {
 
 	logger := logrus.New()
@@ -42,6 +47,8 @@ func NewNullLogger() (*logrus.Logger, *Hook) {
 }
 
 func (t *Hook) Fire(e *logrus.Entry) error {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 	t.Entries = append(t.Entries, e)
 	return nil
 }
@@ -51,17 +58,35 @@ func (t *Hook) Levels() []logrus.Level {
 }
 
 // LastEntry returns the last entry that was logged or nil.
-func (t *Hook) LastEntry() (l *logrus.Entry) {
+func (t *Hook) LastEntry() *logrus.Entry {
-
+	t.mu.RLock()
-	if i := len(t.Entries) - 1; i < 0 {
+	defer t.mu.RUnlock()
+	i := len(t.Entries) - 1
+	if i < 0 {
 		return nil
-	} else {
-		return t.Entries[i]
 	}
+	// Make a copy, for safety
+	e := *t.Entries[i]
+	return &e
+}
 
+// AllEntries returns all entries that were logged.
+func (t *Hook) AllEntries() []*logrus.Entry {
+	t.mu.RLock()
+	defer t.mu.RUnlock()
+	// Make a copy so the returned value won't race with future log requests
+	entries := make([]*logrus.Entry, len(t.Entries))
+	for i, entry := range t.Entries {
+		// Make a copy, for safety
+		e := *entry
+		entries[i] = &e
+	}
+	return entries
 }
 
 // Reset removes all Entries from this test hook.
 func (t *Hook) Reset() {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 	t.Entries = make([]*logrus.Entry, 0)
 }