From eb0da7f9cb48eb98cd8f239c1a5cc7ca0ee501cd Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 25 Sep 2018 14:43:28 +0400 Subject: [PATCH] libs: Handle SIGHUP explicitly inside autofile (#2480) * handle SIGHUP explicitly inside autofile Refs #2260 * libs: Use consistent channel suffix --- libs/autofile/autofile.go | 79 +++++++++++++++++++++------------ libs/autofile/autofile_test.go | 9 +--- libs/autofile/sighup_watcher.go | 69 ---------------------------- 3 files changed, 52 insertions(+), 105 deletions(-) delete mode 100644 libs/autofile/sighup_watcher.go diff --git a/libs/autofile/autofile.go b/libs/autofile/autofile.go index fa1eab20b..6822545e2 100644 --- a/libs/autofile/autofile.go +++ b/libs/autofile/autofile.go @@ -2,7 +2,9 @@ package autofile import ( "os" + "os/signal" "sync" + "syscall" "time" cmn "github.com/tendermint/tendermint/libs/common" @@ -32,50 +34,70 @@ if err != nil { */ const ( - autoFileOpenDuration = 1000 * time.Millisecond - autoFilePerms = os.FileMode(0600) + autoFileClosePeriod = 1000 * time.Millisecond + autoFilePerms = os.FileMode(0600) ) -// Automatically closes and re-opens file for writing. +// AutoFile automatically closes and re-opens file for writing. The file is +// automatically setup to close itself every 1s and upon receiving SIGHUP. +// // This is useful for using a log file with the logrotate tool. type AutoFile struct { - ID string - Path string - ticker *time.Ticker - tickerStopped chan struct{} // closed when ticker is stopped - mtx sync.Mutex - file *os.File + ID string + Path string + + closeTicker *time.Ticker + closeTickerStopc chan struct{} // closed when closeTicker is stopped + hupc chan os.Signal + + mtx sync.Mutex + file *os.File } -func OpenAutoFile(path string) (af *AutoFile, err error) { - af = &AutoFile{ - ID: cmn.RandStr(12) + ":" + path, - Path: path, - ticker: time.NewTicker(autoFileOpenDuration), - tickerStopped: make(chan struct{}), +// OpenAutoFile creates an AutoFile in the path (with random ID). If there is +// an error, it will be of type *PathError or *ErrPermissionsChanged (if file's +// permissions got changed (should be 0600)). +func OpenAutoFile(path string) (*AutoFile, error) { + af := &AutoFile{ + ID: cmn.RandStr(12) + ":" + path, + Path: path, + closeTicker: time.NewTicker(autoFileClosePeriod), + closeTickerStopc: make(chan struct{}), } - if err = af.openFile(); err != nil { - return + if err := af.openFile(); err != nil { + af.Close() + return nil, err } - go af.processTicks() - sighupWatchers.addAutoFile(af) - return + + // Close file on SIGHUP. + af.hupc = make(chan os.Signal, 1) + signal.Notify(af.hupc, syscall.SIGHUP) + go func() { + for range af.hupc { + af.closeFile() + } + }() + + go af.closeFileRoutine() + + return af, nil } func (af *AutoFile) Close() error { - af.ticker.Stop() - close(af.tickerStopped) - err := af.closeFile() - sighupWatchers.removeAutoFile(af) - return err + af.closeTicker.Stop() + close(af.closeTickerStopc) + if af.hupc != nil { + close(af.hupc) + } + return af.closeFile() } -func (af *AutoFile) processTicks() { +func (af *AutoFile) closeFileRoutine() { for { select { - case <-af.ticker.C: + case <-af.closeTicker.C: af.closeFile() - case <-af.tickerStopped: + case <-af.closeTickerStopc: return } } @@ -89,6 +111,7 @@ func (af *AutoFile) closeFile() (err error) { if file == nil { return nil } + af.file = nil return file.Close() } diff --git a/libs/autofile/autofile_test.go b/libs/autofile/autofile_test.go index e8a9b3e4d..5408d8204 100644 --- a/libs/autofile/autofile_test.go +++ b/libs/autofile/autofile_test.go @@ -3,7 +3,6 @@ package autofile import ( "io/ioutil" "os" - "sync/atomic" "syscall" "testing" "time" @@ -37,13 +36,10 @@ func TestSIGHUP(t *testing.T) { require.NoError(t, err) // Send SIGHUP to self. - oldSighupCounter := atomic.LoadInt32(&sighupCounter) syscall.Kill(syscall.Getpid(), syscall.SIGHUP) // Wait a bit... signals are not handled synchronously. - for atomic.LoadInt32(&sighupCounter) == oldSighupCounter { - time.Sleep(time.Millisecond * 10) - } + time.Sleep(time.Millisecond * 10) // Write more to the file. _, err = af.Write([]byte("Line 3\n")) @@ -87,7 +83,4 @@ func TestOpenAutoFilePerms(t *testing.T) { } else { t.Errorf("unexpected error %v", e) } - - err = af.Close() - require.NoError(t, err) } diff --git a/libs/autofile/sighup_watcher.go b/libs/autofile/sighup_watcher.go deleted file mode 100644 index f72f12fcd..000000000 --- a/libs/autofile/sighup_watcher.go +++ /dev/null @@ -1,69 +0,0 @@ -package autofile - -import ( - "os" - "os/signal" - "sync" - "sync/atomic" - "syscall" -) - -func init() { - initSighupWatcher() -} - -var sighupWatchers *SighupWatcher -var sighupCounter int32 // For testing - -func initSighupWatcher() { - sighupWatchers = newSighupWatcher() - - hup := make(chan os.Signal, 1) - signal.Notify(hup, syscall.SIGHUP) - - quit := make(chan os.Signal, 1) - signal.Notify(quit, os.Interrupt, syscall.SIGTERM) - - go func() { - select { - case <-hup: - sighupWatchers.closeAll() - atomic.AddInt32(&sighupCounter, 1) - case <-quit: - return - } - }() -} - -// Watchces for SIGHUP events and notifies registered AutoFiles -type SighupWatcher struct { - mtx sync.Mutex - autoFiles map[string]*AutoFile -} - -func newSighupWatcher() *SighupWatcher { - return &SighupWatcher{ - autoFiles: make(map[string]*AutoFile, 10), - } -} - -func (w *SighupWatcher) addAutoFile(af *AutoFile) { - w.mtx.Lock() - w.autoFiles[af.ID] = af - w.mtx.Unlock() -} - -// If AutoFile isn't registered or was already removed, does nothing. -func (w *SighupWatcher) removeAutoFile(af *AutoFile) { - w.mtx.Lock() - delete(w.autoFiles, af.ID) - w.mtx.Unlock() -} - -func (w *SighupWatcher) closeAll() { - w.mtx.Lock() - for _, af := range w.autoFiles { - af.closeFile() - } - w.mtx.Unlock() -}