From a6a67ea9b28257fb18026c2d8ec5430edb7d2afd Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Wed, 26 Oct 2016 21:50:28 -0700 Subject: [PATCH 1/5] Remove AutoFile; Use go-autofile instead --- os.go | 154 ---------------------------------------------------------- 1 file changed, 154 deletions(-) diff --git a/os.go b/os.go index a273bec48..9f38027fa 100644 --- a/os.go +++ b/os.go @@ -7,19 +7,12 @@ import ( "os" "os/signal" "strings" - "sync" - "syscall" - "time" ) var ( GoPath = os.Getenv("GOPATH") ) -func init() { - initAFSIGHUPWatcher() -} - func TrapSignal(cb func()) { c := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt) @@ -112,153 +105,6 @@ func WriteFileAtomic(filePath string, newBytes []byte, mode os.FileMode) error { //-------------------------------------------------------------------------------- -/* AutoFile usage - -// Create/Append to ./autofile_test -af, err := OpenAutoFile("autofile_test") -if err != nil { - panic(err) -} - -// Stream of writes. -// During this time, the file may be moved e.g. by logRotate. -for i := 0; i < 60; i++ { - af.Write([]byte(Fmt("LOOP(%v)", i))) - time.Sleep(time.Second) -} - -// Close the AutoFile -err = af.Close() -if err != nil { - panic(err) -} -*/ - -const autoFileOpenDuration = 1000 * time.Millisecond - -// Automatically closes and re-opens file for writing. -// This is useful for using a log file with the logrotate tool. -type AutoFile struct { - ID string - Path string - ticker *time.Ticker - mtx sync.Mutex - file *os.File -} - -func OpenAutoFile(path string) (af *AutoFile, err error) { - af = &AutoFile{ - ID: RandStr(12) + ":" + path, - Path: path, - ticker: time.NewTicker(autoFileOpenDuration), - } - if err = af.openFile(); err != nil { - return - } - go af.processTicks() - autoFileWatchers.addAutoFile(af) - return -} - -func (af *AutoFile) Close() error { - af.ticker.Stop() - err := af.closeFile() - autoFileWatchers.removeAutoFile(af) - return err -} - -func (af *AutoFile) processTicks() { - for { - _, ok := <-af.ticker.C - if !ok { - return // Done. - } - af.closeFile() - } -} - -func (af *AutoFile) closeFile() (err error) { - af.mtx.Lock() - defer af.mtx.Unlock() - - file := af.file - if file == nil { - return nil - } - af.file = nil - return file.Close() -} - -func (af *AutoFile) Write(b []byte) (n int, err error) { - af.mtx.Lock() - defer af.mtx.Unlock() - if af.file == nil { - if err = af.openFile(); err != nil { - return - } - } - return af.file.Write(b) -} - -func (af *AutoFile) openFile() error { - file, err := os.OpenFile(af.Path, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600) - if err != nil { - return err - } - af.file = file - return nil -} - -//-------------------------------------------------------------------------------- - -var autoFileWatchers *afSIGHUPWatcher - -func initAFSIGHUPWatcher() { - autoFileWatchers = newAFSIGHUPWatcher() - - c := make(chan os.Signal, 1) - signal.Notify(c, syscall.SIGHUP) - - go func() { - for _ = range c { - autoFileWatchers.closeAll() - } - }() -} - -type afSIGHUPWatcher struct { - mtx sync.Mutex - autoFiles map[string]*AutoFile -} - -func newAFSIGHUPWatcher() *afSIGHUPWatcher { - return &afSIGHUPWatcher{ - autoFiles: make(map[string]*AutoFile, 10), - } -} - -func (afw *afSIGHUPWatcher) addAutoFile(af *AutoFile) { - afw.mtx.Lock() - afw.autoFiles[af.ID] = af - afw.mtx.Unlock() -} - -func (afw *afSIGHUPWatcher) removeAutoFile(af *AutoFile) { - afw.mtx.Lock() - delete(afw.autoFiles, af.ID) - afw.mtx.Unlock() -} - -func (afw *afSIGHUPWatcher) closeAll() { - afw.mtx.Lock() - for _, af := range afw.autoFiles { - af.closeFile() - } - afw.mtx.Unlock() -} - -//-------------------------------------------------------------------------------- - func Tempfile(prefix string) (*os.File, string) { file, err := ioutil.TempFile("", prefix) if err != nil { From 25dc9ae3451db5b03bcf1c02bd3559094101e402 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Fri, 28 Oct 2016 12:09:22 -0700 Subject: [PATCH 2/5] QuitService->BaseService --- service.go | 29 ++++++++++++++--------------- service_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 service_test.go diff --git a/service.go b/service.go index 86ef20ead..7336446ff 100644 --- a/service.go +++ b/service.go @@ -65,6 +65,7 @@ type BaseService struct { name string started uint32 // atomic stopped uint32 // atomic + Quit chan struct{} // The "subclass" of BaseService impl Service @@ -74,6 +75,7 @@ func NewBaseService(log log15.Logger, name string, impl Service) *BaseService { return &BaseService{ log: log, name: name, + Quit: make(chan struct{}), impl: impl, } } @@ -102,6 +104,8 @@ func (bs *BaseService) Start() (bool, error) { } // Implements Service +// NOTE: Do not put anything in here, +// that way users don't need to call BaseService.OnStart() func (bs *BaseService) OnStart() error { return nil } // Implements Service @@ -111,6 +115,7 @@ func (bs *BaseService) Stop() bool { bs.log.Info(Fmt("Stopping %v", bs.name), "impl", bs.impl) } bs.impl.OnStop() + close(bs.Quit) return true } else { if bs.log != nil { @@ -121,6 +126,8 @@ func (bs *BaseService) Stop() bool { } // Implements Service +// NOTE: Do not put anything in here, +// that way users don't need to call BaseService.OnStop() func (bs *BaseService) OnStop() {} // Implements Service @@ -151,6 +158,10 @@ func (bs *BaseService) IsRunning() bool { return atomic.LoadUint32(&bs.started) == 1 && atomic.LoadUint32(&bs.stopped) == 0 } +func (bs *QuitService) Wait() { + <-bs.Quit +} + // Implements Servce func (bs *BaseService) String() string { return bs.name @@ -160,25 +171,13 @@ func (bs *BaseService) String() string { type QuitService struct { BaseService - Quit chan struct{} } func NewQuitService(log log15.Logger, name string, impl Service) *QuitService { + if log != nil { + log.Warn("QuitService is deprecated, use BaseService instead") + } return &QuitService{ BaseService: *NewBaseService(log, name, impl), - Quit: nil, - } -} - -// NOTE: when overriding OnStart, must call .QuitService.OnStart(). -func (qs *QuitService) OnStart() error { - qs.Quit = make(chan struct{}) - return nil -} - -// NOTE: when overriding OnStop, must call .QuitService.OnStop(). -func (qs *QuitService) OnStop() { - if qs.Quit != nil { - close(qs.Quit) } } diff --git a/service_test.go b/service_test.go new file mode 100644 index 000000000..6e24dad6a --- /dev/null +++ b/service_test.go @@ -0,0 +1,24 @@ +package common + +import ( + "testing" +) + +func TestBaseServiceWait(t *testing.T) { + + type TestService struct { + BaseService + } + ts := &TestService{} + ts.BaseService = *NewBaseService(nil, "TestService", ts) + ts.Start() + + go func() { + ts.Stop() + }() + + for i := 0; i < 10; i++ { + ts.Wait() + } + +} From 890e24073036b6dffb7c56d8980545d7d660026b Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Fri, 28 Oct 2016 12:09:34 -0700 Subject: [PATCH 3/5] Remove AutoFile tests --- os_test.go | 64 ------------------------------------------------------ service.go | 2 +- 2 files changed, 1 insertion(+), 65 deletions(-) delete mode 100644 os_test.go diff --git a/os_test.go b/os_test.go deleted file mode 100644 index c0effdc2b..000000000 --- a/os_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package common - -import ( - "os" - "syscall" - "testing" -) - -func TestSIGHUP(t *testing.T) { - - // First, create an AutoFile writing to a tempfile dir - file, name := Tempfile("sighup_test") - err := file.Close() - if err != nil { - t.Fatalf("Error creating tempfile: %v", err) - } - // Here is the actual AutoFile - af, err := OpenAutoFile(name) - if err != nil { - t.Fatalf("Error creating autofile: %v", err) - } - - // Write to the file. - _, err = af.Write([]byte("Line 1\n")) - if err != nil { - t.Fatalf("Error writing to autofile: %v", err) - } - _, err = af.Write([]byte("Line 2\n")) - if err != nil { - t.Fatalf("Error writing to autofile: %v", err) - } - - // Send SIGHUP to self. - syscall.Kill(syscall.Getpid(), syscall.SIGHUP) - - // Move the file over - err = os.Rename(name, name+"_old") - if err != nil { - t.Fatalf("Error moving autofile: %v", err) - } - - // Write more to the file. - _, err = af.Write([]byte("Line 3\n")) - if err != nil { - t.Fatalf("Error writing to autofile: %v", err) - } - _, err = af.Write([]byte("Line 4\n")) - if err != nil { - t.Fatalf("Error writing to autofile: %v", err) - } - err = af.Close() - if err != nil { - t.Fatalf("Error closing autofile") - } - - // Both files should exist - if body := MustReadFile(name + "_old"); string(body) != "Line 1\nLine 2\n" { - t.Errorf("Unexpected body %s", body) - } - if body := MustReadFile(name); string(body) != "Line 3\nLine 4\n" { - t.Errorf("Unexpected body %s", body) - } - -} diff --git a/service.go b/service.go index 7336446ff..e2d31925b 100644 --- a/service.go +++ b/service.go @@ -158,7 +158,7 @@ func (bs *BaseService) IsRunning() bool { return atomic.LoadUint32(&bs.started) == 1 && atomic.LoadUint32(&bs.stopped) == 0 } -func (bs *QuitService) Wait() { +func (bs *BaseService) Wait() { <-bs.Quit } From f40b1b65f81b695b44bae7ffe8f98c485b11846e Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Mon, 21 Nov 2016 20:01:11 -0800 Subject: [PATCH 4/5] Add Tempdir --- os.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/os.go b/os.go index 9f38027fa..e8943c0c5 100644 --- a/os.go +++ b/os.go @@ -113,6 +113,19 @@ func Tempfile(prefix string) (*os.File, string) { return file, file.Name() } +func Tempdir(prefix string) (*os.File, string) { + tempDir := os.TempDir() + "/" + prefix + RandStr(12) + err := EnsureDir(tempDir, 0700) + if err != nil { + panic(Fmt("Error creating temp dir: %v", err)) + } + dir, err := os.Open(tempDir) + if err != nil { + panic(Fmt("Error opening temp dir: %v", err)) + } + return dir, tempDir +} + //-------------------------------------------------------------------------------- func Prompt(prompt string, defaultValue string) (string, error) { From a552e49b501bd438ed0055e180371b452aa2cfed Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 12 Dec 2016 23:08:02 -0500 Subject: [PATCH 5/5] Reverts commit f40b1b to a6a67e --- os.go | 165 ++++++++++++++++++++++++++++++++++++++++++++---- os_test.go | 64 +++++++++++++++++++ service.go | 29 +++++---- service_test.go | 24 ------- 4 files changed, 232 insertions(+), 50 deletions(-) create mode 100644 os_test.go delete mode 100644 service_test.go diff --git a/os.go b/os.go index e8943c0c5..a273bec48 100644 --- a/os.go +++ b/os.go @@ -7,12 +7,19 @@ import ( "os" "os/signal" "strings" + "sync" + "syscall" + "time" ) var ( GoPath = os.Getenv("GOPATH") ) +func init() { + initAFSIGHUPWatcher() +} + func TrapSignal(cb func()) { c := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt) @@ -105,25 +112,159 @@ func WriteFileAtomic(filePath string, newBytes []byte, mode os.FileMode) error { //-------------------------------------------------------------------------------- -func Tempfile(prefix string) (*os.File, string) { - file, err := ioutil.TempFile("", prefix) - if err != nil { - PanicCrisis(err) +/* AutoFile usage + +// Create/Append to ./autofile_test +af, err := OpenAutoFile("autofile_test") +if err != nil { + panic(err) +} + +// Stream of writes. +// During this time, the file may be moved e.g. by logRotate. +for i := 0; i < 60; i++ { + af.Write([]byte(Fmt("LOOP(%v)", i))) + time.Sleep(time.Second) +} + +// Close the AutoFile +err = af.Close() +if err != nil { + panic(err) +} +*/ + +const autoFileOpenDuration = 1000 * time.Millisecond + +// Automatically closes and re-opens file for writing. +// This is useful for using a log file with the logrotate tool. +type AutoFile struct { + ID string + Path string + ticker *time.Ticker + mtx sync.Mutex + file *os.File +} + +func OpenAutoFile(path string) (af *AutoFile, err error) { + af = &AutoFile{ + ID: RandStr(12) + ":" + path, + Path: path, + ticker: time.NewTicker(autoFileOpenDuration), } - return file, file.Name() + if err = af.openFile(); err != nil { + return + } + go af.processTicks() + autoFileWatchers.addAutoFile(af) + return +} + +func (af *AutoFile) Close() error { + af.ticker.Stop() + err := af.closeFile() + autoFileWatchers.removeAutoFile(af) + return err +} + +func (af *AutoFile) processTicks() { + for { + _, ok := <-af.ticker.C + if !ok { + return // Done. + } + af.closeFile() + } +} + +func (af *AutoFile) closeFile() (err error) { + af.mtx.Lock() + defer af.mtx.Unlock() + + file := af.file + if file == nil { + return nil + } + af.file = nil + return file.Close() +} + +func (af *AutoFile) Write(b []byte) (n int, err error) { + af.mtx.Lock() + defer af.mtx.Unlock() + if af.file == nil { + if err = af.openFile(); err != nil { + return + } + } + return af.file.Write(b) } -func Tempdir(prefix string) (*os.File, string) { - tempDir := os.TempDir() + "/" + prefix + RandStr(12) - err := EnsureDir(tempDir, 0700) +func (af *AutoFile) openFile() error { + file, err := os.OpenFile(af.Path, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600) if err != nil { - panic(Fmt("Error creating temp dir: %v", err)) + return err } - dir, err := os.Open(tempDir) + af.file = file + return nil +} + +//-------------------------------------------------------------------------------- + +var autoFileWatchers *afSIGHUPWatcher + +func initAFSIGHUPWatcher() { + autoFileWatchers = newAFSIGHUPWatcher() + + c := make(chan os.Signal, 1) + signal.Notify(c, syscall.SIGHUP) + + go func() { + for _ = range c { + autoFileWatchers.closeAll() + } + }() +} + +type afSIGHUPWatcher struct { + mtx sync.Mutex + autoFiles map[string]*AutoFile +} + +func newAFSIGHUPWatcher() *afSIGHUPWatcher { + return &afSIGHUPWatcher{ + autoFiles: make(map[string]*AutoFile, 10), + } +} + +func (afw *afSIGHUPWatcher) addAutoFile(af *AutoFile) { + afw.mtx.Lock() + afw.autoFiles[af.ID] = af + afw.mtx.Unlock() +} + +func (afw *afSIGHUPWatcher) removeAutoFile(af *AutoFile) { + afw.mtx.Lock() + delete(afw.autoFiles, af.ID) + afw.mtx.Unlock() +} + +func (afw *afSIGHUPWatcher) closeAll() { + afw.mtx.Lock() + for _, af := range afw.autoFiles { + af.closeFile() + } + afw.mtx.Unlock() +} + +//-------------------------------------------------------------------------------- + +func Tempfile(prefix string) (*os.File, string) { + file, err := ioutil.TempFile("", prefix) if err != nil { - panic(Fmt("Error opening temp dir: %v", err)) + PanicCrisis(err) } - return dir, tempDir + return file, file.Name() } //-------------------------------------------------------------------------------- diff --git a/os_test.go b/os_test.go new file mode 100644 index 000000000..c0effdc2b --- /dev/null +++ b/os_test.go @@ -0,0 +1,64 @@ +package common + +import ( + "os" + "syscall" + "testing" +) + +func TestSIGHUP(t *testing.T) { + + // First, create an AutoFile writing to a tempfile dir + file, name := Tempfile("sighup_test") + err := file.Close() + if err != nil { + t.Fatalf("Error creating tempfile: %v", err) + } + // Here is the actual AutoFile + af, err := OpenAutoFile(name) + if err != nil { + t.Fatalf("Error creating autofile: %v", err) + } + + // Write to the file. + _, err = af.Write([]byte("Line 1\n")) + if err != nil { + t.Fatalf("Error writing to autofile: %v", err) + } + _, err = af.Write([]byte("Line 2\n")) + if err != nil { + t.Fatalf("Error writing to autofile: %v", err) + } + + // Send SIGHUP to self. + syscall.Kill(syscall.Getpid(), syscall.SIGHUP) + + // Move the file over + err = os.Rename(name, name+"_old") + if err != nil { + t.Fatalf("Error moving autofile: %v", err) + } + + // Write more to the file. + _, err = af.Write([]byte("Line 3\n")) + if err != nil { + t.Fatalf("Error writing to autofile: %v", err) + } + _, err = af.Write([]byte("Line 4\n")) + if err != nil { + t.Fatalf("Error writing to autofile: %v", err) + } + err = af.Close() + if err != nil { + t.Fatalf("Error closing autofile") + } + + // Both files should exist + if body := MustReadFile(name + "_old"); string(body) != "Line 1\nLine 2\n" { + t.Errorf("Unexpected body %s", body) + } + if body := MustReadFile(name); string(body) != "Line 3\nLine 4\n" { + t.Errorf("Unexpected body %s", body) + } + +} diff --git a/service.go b/service.go index e2d31925b..86ef20ead 100644 --- a/service.go +++ b/service.go @@ -65,7 +65,6 @@ type BaseService struct { name string started uint32 // atomic stopped uint32 // atomic - Quit chan struct{} // The "subclass" of BaseService impl Service @@ -75,7 +74,6 @@ func NewBaseService(log log15.Logger, name string, impl Service) *BaseService { return &BaseService{ log: log, name: name, - Quit: make(chan struct{}), impl: impl, } } @@ -104,8 +102,6 @@ func (bs *BaseService) Start() (bool, error) { } // Implements Service -// NOTE: Do not put anything in here, -// that way users don't need to call BaseService.OnStart() func (bs *BaseService) OnStart() error { return nil } // Implements Service @@ -115,7 +111,6 @@ func (bs *BaseService) Stop() bool { bs.log.Info(Fmt("Stopping %v", bs.name), "impl", bs.impl) } bs.impl.OnStop() - close(bs.Quit) return true } else { if bs.log != nil { @@ -126,8 +121,6 @@ func (bs *BaseService) Stop() bool { } // Implements Service -// NOTE: Do not put anything in here, -// that way users don't need to call BaseService.OnStop() func (bs *BaseService) OnStop() {} // Implements Service @@ -158,10 +151,6 @@ func (bs *BaseService) IsRunning() bool { return atomic.LoadUint32(&bs.started) == 1 && atomic.LoadUint32(&bs.stopped) == 0 } -func (bs *BaseService) Wait() { - <-bs.Quit -} - // Implements Servce func (bs *BaseService) String() string { return bs.name @@ -171,13 +160,25 @@ func (bs *BaseService) String() string { type QuitService struct { BaseService + Quit chan struct{} } func NewQuitService(log log15.Logger, name string, impl Service) *QuitService { - if log != nil { - log.Warn("QuitService is deprecated, use BaseService instead") - } return &QuitService{ BaseService: *NewBaseService(log, name, impl), + Quit: nil, + } +} + +// NOTE: when overriding OnStart, must call .QuitService.OnStart(). +func (qs *QuitService) OnStart() error { + qs.Quit = make(chan struct{}) + return nil +} + +// NOTE: when overriding OnStop, must call .QuitService.OnStop(). +func (qs *QuitService) OnStop() { + if qs.Quit != nil { + close(qs.Quit) } } diff --git a/service_test.go b/service_test.go deleted file mode 100644 index 6e24dad6a..000000000 --- a/service_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package common - -import ( - "testing" -) - -func TestBaseServiceWait(t *testing.T) { - - type TestService struct { - BaseService - } - ts := &TestService{} - ts.BaseService = *NewBaseService(nil, "TestService", ts) - ts.Start() - - go func() { - ts.Stop() - }() - - for i := 0; i < 10; i++ { - ts.Wait() - } - -}