Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

Commit

Permalink
Decouple removing the fileWatcher from reading
Browse files Browse the repository at this point in the history
Fixes #27779

Currently `followLogs` can get into a deadlock if we receive an inotify
IN_MODIFY event while we are trying to close the `fileWatcher`. This is
due to the fact that closing the `fileWatcher` happens in the same block
as consumes events from the `fileWatcher`. We are trying to run
`fileWatcher.Close`, which is waiting for an IN_IGNORE event to come in
over inotify to confirm the watch was been removed. But, because an
IN_MODIFY event has appeared after `Close` was entered but before the
IN_IGNORE, the broadcast never comes. The IN_MODIFY cannot be consumed
as the events channel is unbuffered and the only `select` that reads
from it is busy waiting for the IN_IGNORE event.

In order to try and fix this race condition I've moved the removal of
the `fileWatcher` out to a separate go block that waits for a signal to
close, removes the watcher and then signals to the previous selects on
the close signal.

This has introduced a `fileWatcher.Remove` in the final case, but if we
try and remove a watcher that does not exist it will just return an
error saying so. We are not doing any checking on the return of `Remove`
so this shouldn't cause any side-effects.

Signed-off-by: Tom Booth <tombooth@gmail.com>
  • Loading branch information
tombooth committed Oct 28, 2016
1 parent 6314bec commit a69a59f
Showing 1 changed file with 17 additions and 5 deletions.
22 changes: 17 additions & 5 deletions daemon/logger/jsonfilelog/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"time"

"golang.org/x/net/context"
"gopkg.in/fsnotify.v1"

"github.com/Sirupsen/logrus"
Expand Down Expand Up @@ -172,9 +173,22 @@ func followLogs(f *os.File, logWatcher *logger.LogWatcher, notifyRotate chan int
}
defer func() {
f.Close()
fileWatcher.Remove(name)
fileWatcher.Close()
}()

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go func() {
select {
case <-logWatcher.WatchClose():
fileWatcher.Remove(name)
cancel()
case <-ctx.Done():
return
}
}()

var retries int
handleRotate := func() error {
f.Close()
Expand Down Expand Up @@ -209,8 +223,7 @@ func followLogs(f *os.File, logWatcher *logger.LogWatcher, notifyRotate chan int
case fsnotify.Rename, fsnotify.Remove:
select {
case <-notifyRotate:
case <-logWatcher.WatchClose():
fileWatcher.Remove(name)
case <-ctx.Done():
return errDone
}
if err := handleRotate(); err != nil {
Expand All @@ -232,8 +245,7 @@ func followLogs(f *os.File, logWatcher *logger.LogWatcher, notifyRotate chan int
return errRetry
}
return err
case <-logWatcher.WatchClose():
fileWatcher.Remove(name)
case <-ctx.Done():
return errDone
}
}
Expand Down Expand Up @@ -290,7 +302,7 @@ func followLogs(f *os.File, logWatcher *logger.LogWatcher, notifyRotate chan int
}
select {
case logWatcher.Msg <- msg:
case <-logWatcher.WatchClose():
case <-ctx.Done():
logWatcher.Msg <- msg
for {
msg, err := decodeLogLine(dec, l)
Expand Down

0 comments on commit a69a59f

Please sign in to comment.