diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index da19405c2bd9..187a624b213f 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,6 +31,7 @@ https://github.com/elastic/beats/compare/v6.0.0-alpha1...master[Check the HEAD d *Affecting all Beats* - Don't stop with error loading the ES template if the ES output is not enabled. {pull}4436[4436] +- Fix race condition in internal logging rotator. {pull}4519[4519] *Filebeat* diff --git a/libbeat/logp/file_rotator.go b/libbeat/logp/file_rotator.go index 0cb05077d59b..c7db4d7acd60 100644 --- a/libbeat/logp/file_rotator.go +++ b/libbeat/logp/file_rotator.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" ) const RotatorMaxFiles = 1024 @@ -21,6 +22,7 @@ type FileRotator struct { current *os.File currentSize uint64 + currentLock sync.RWMutex } func (rotator *FileRotator) CreateDirectory() error { @@ -73,16 +75,26 @@ func (rotator *FileRotator) WriteLine(line []byte) error { } line = append(line, '\n') + + rotator.currentLock.RLock() _, err := rotator.current.Write(line) + rotator.currentLock.RUnlock() + if err != nil { return err } + + rotator.currentLock.Lock() rotator.currentSize += uint64(len(line)) + rotator.currentLock.Unlock() return nil } func (rotator *FileRotator) shouldRotate() bool { + rotator.currentLock.RLock() + defer rotator.currentLock.RUnlock() + if rotator.current == nil { return true } @@ -112,6 +124,8 @@ func (rotator *FileRotator) FileExists(fileNo int) bool { } func (rotator *FileRotator) Rotate() error { + rotator.currentLock.Lock() + defer rotator.currentLock.Unlock() if rotator.current != nil { if err := rotator.current.Close(); err != nil { diff --git a/libbeat/logp/file_rotator_test.go b/libbeat/logp/file_rotator_test.go index d4d82805802a..898d8e4f980d 100644 --- a/libbeat/logp/file_rotator_test.go +++ b/libbeat/logp/file_rotator_test.go @@ -174,3 +174,32 @@ func TestConfigSane(t *testing.T) { } assert.NotNil(t, rotator.CheckIfConfigSane()) } + +func TestRaceConditions(t *testing.T) { + // Make sure concurrent `WriteLine` calls don't end up in race conditions around `rotator.current` + if testing.Verbose() { + LogInit(LOG_DEBUG, "", false, true, []string{"rotator"}) + } + + dir, err := ioutil.TempDir("", "test_rotator_") + if err != nil { + t.Errorf("Error: %s", err.Error()) + return + } + + Debug("rotator", "Directory: %s", dir) + + rotateeverybytes := uint64(10) + keepfiles := 20 + + rotator := FileRotator{ + Path: dir, + Name: "testbeat", + RotateEveryBytes: &rotateeverybytes, + KeepFiles: &keepfiles, + } + + for i := 0; i < 1000; i++ { + go rotator.WriteLine([]byte(string(i))) + } +}