Skip to content

Commit

Permalink
Fix race condition in logging
Browse files Browse the repository at this point in the history
`logp.FileRotator` had an issue when `WriteLines` is called concurrently
  • Loading branch information
exekias committed Jun 16, 2017
1 parent f12f09c commit cdea7f0
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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*

Expand Down
14 changes: 14 additions & 0 deletions libbeat/logp/file_rotator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path/filepath"
"strconv"
"strings"
"sync"
)

const RotatorMaxFiles = 1024
Expand All @@ -21,6 +22,7 @@ type FileRotator struct {

current *os.File
currentSize uint64
currentLock sync.RWMutex
}

func (rotator *FileRotator) CreateDirectory() error {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 29 additions & 0 deletions libbeat/logp/file_rotator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
}

0 comments on commit cdea7f0

Please sign in to comment.