Merge pull request #5409 from exekias/backport_4519_5.6

Cherry-pick #4519 to 5.6: Fix race condition in logging

CHANGELOG.asciidoc

@@ -28,6 +28,8 @@ https://github.com/elastic/beats/compare/v5.6.3...5.6[Check the HEAD diff]
 
 *Affecting all Beats*
 
+- Fix race condition in internal logging rotator. {pull}4519[4519]
+
 *Filebeat*
 
 *Heartbeat*

libbeat/logp/file_rotator.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 )
 
 const RotatorMaxFiles = 1024
@@ -20,6 +21,7 @@ type FileRotator struct {
 
 	current     *os.File
 	currentSize uint64
+	currentLock sync.RWMutex
 }
 
 func (rotator *FileRotator) CreateDirectory() error {
@@ -68,16 +70,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
 	}
@@ -107,6 +119,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 {

libbeat/logp/file_rotator_test.go

@@ -161,3 +161,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)))
+	}
+}