diff --git a/global.go b/global.go index e069cd99d..52968ed68 100644 --- a/global.go +++ b/global.go @@ -29,9 +29,6 @@ import ( var ( // L is a global Logger. It defaults to a no-op implementation but can be // replaced using ReplaceGlobals. - // - // Both L and S are unsynchronized, so replacing them while they're in - // use isn't safe. L = New(nil) // S is a global SugaredLogger, similar to L. It also defaults to a no-op // implementation. @@ -40,11 +37,9 @@ var ( // ReplaceGlobals replaces the global Logger L and the global SugaredLogger S, // and returns a function to restore the original values. -// -// Note that replacing the global loggers isn't safe while they're being used; -// in practice, this means that only the owner of the application's main -// function should use this method. func ReplaceGlobals(logger *Logger) func() { + // This doesn't require synchronization to be safe, since pointers are a + // single word. prev := *L L = logger S = logger.Sugar() diff --git a/global_test.go b/global_test.go index e10ccb141..6d08cc0cd 100644 --- a/global_test.go +++ b/global_test.go @@ -22,11 +22,15 @@ package zap import ( "log" + "sync" "testing" + "time" "github.com/stretchr/testify/assert" + "go.uber.org/atomic" "go.uber.org/zap/internal/observer" + "go.uber.org/zap/testutils" "go.uber.org/zap/zapcore" ) @@ -59,6 +63,34 @@ func TestReplaceGlobals(t *testing.T) { assert.Equal(t, initialS, *S, "Expected func returned from ReplaceGlobals to restore initial S.") } +func TestGlobalsConcurrentUse(t *testing.T) { + var ( + stop atomic.Bool + wg sync.WaitGroup + ) + + for i := 0; i < 100; i++ { + wg.Add(2) + go func() { + for stop.Load() { + ReplaceGlobals(New(nil)) + } + wg.Done() + }() + go func() { + for stop.Load() { + L.Info("") + S.Info("") + } + wg.Done() + }() + } + + testutils.Sleep(100 * time.Millisecond) + stop.Toggle() + wg.Wait() +} + func TestRedirectStdLog(t *testing.T) { initialFlags := log.Flags() initialPrefix := log.Prefix()