From 9faa37b6eaafc86a11c36454fc12235c7fc0066d Mon Sep 17 00:00:00 2001 From: sam boyer Date: Tue, 10 Jan 2017 22:34:51 -0500 Subject: [PATCH] Use default gps signal handling --- lock.json | 2 +- remove.go | 1 + status.go | 1 + vendor/github.com/sdboyer/gps/manager_test.go | 15 +--- .../github.com/sdboyer/gps/source_manager.go | 71 +++++++++---------- 5 files changed, 38 insertions(+), 52 deletions(-) diff --git a/lock.json b/lock.json index e5411840e2..7dad965a04 100644 --- a/lock.json +++ b/lock.json @@ -36,7 +36,7 @@ { "name": "github.com/sdboyer/gps", "branch": "master", - "revision": "88027dbe5e8fb2a2764036d1858f498a9aa6bf1c", + "revision": "669a62fc8b4b2c48757f6ff89aeba9eb8f287a9c", "packages": [ "." ] diff --git a/remove.go b/remove.go index b5e2dc0e6d..3c0f31302c 100644 --- a/remove.go +++ b/remove.go @@ -51,6 +51,7 @@ func (cmd *removeCommand) Run(args []string) error { if err != nil { return err } + sm.UseDefaultSignalHandling() defer sm.Release() cpr, err := depContext.splitAbsoluteProjectRoot(p.absroot) diff --git a/status.go b/status.go index 863a6c543b..1538b7f187 100644 --- a/status.go +++ b/status.go @@ -73,6 +73,7 @@ func (cmd *statusCommand) Run(args []string) error { if err != nil { return err } + sm.UseDefaultSignalHandling() defer sm.Release() if cmd.detailed { diff --git a/vendor/github.com/sdboyer/gps/manager_test.go b/vendor/github.com/sdboyer/gps/manager_test.go index da52df09fd..240124bf03 100644 --- a/vendor/github.com/sdboyer/gps/manager_test.go +++ b/vendor/github.com/sdboyer/gps/manager_test.go @@ -728,9 +728,6 @@ func TestSignalHandling(t *testing.T) { if sm.releasing != 1 { t.Error("Releasing flag did not get set") } - if sm.released != 1 { - t.Error("Released flag did not get set") - } lpath := filepath.Join(sm.cachedir, "sm.lock") if _, err := os.Stat(lpath); err == nil { @@ -739,7 +736,7 @@ func TestSignalHandling(t *testing.T) { clean() sm, clean = mkNaiveSM(t) - SetUpSigHandling(sm) + sm.UseDefaultSignalHandling() go sm.DeduceProjectRoot("rsc.io/pdf") runtime.Gosched() @@ -758,9 +755,6 @@ func TestSignalHandling(t *testing.T) { if sm.releasing != 1 { t.Error("Releasing flag did not get set") } - if sm.released != 1 { - t.Error("Released flag did not get set") - } lpath = filepath.Join(sm.cachedir, "sm.lock") if _, err := os.Stat(lpath); err == nil { @@ -769,9 +763,9 @@ func TestSignalHandling(t *testing.T) { clean() sm, clean = mkNaiveSM(t) - SetUpSigHandling(sm) + sm.UseDefaultSignalHandling() sm.StopSignalHandling() - SetUpSigHandling(sm) + sm.UseDefaultSignalHandling() go sm.DeduceProjectRoot("rsc.io/pdf") //runtime.Gosched() @@ -788,9 +782,6 @@ func TestSignalHandling(t *testing.T) { if sm.releasing != 1 { t.Error("Releasing flag did not get set") } - if sm.released != 1 { - t.Error("Released flag did not get set") - } lpath = filepath.Join(sm.cachedir, "sm.lock") if _, err := os.Stat(lpath); err == nil { diff --git a/vendor/github.com/sdboyer/gps/source_manager.go b/vendor/github.com/sdboyer/gps/source_manager.go index c8634f78c6..0d3be451f8 100644 --- a/vendor/github.com/sdboyer/gps/source_manager.go +++ b/vendor/github.com/sdboyer/gps/source_manager.go @@ -98,8 +98,8 @@ type SourceMgr struct { sigmut sync.Mutex // mutex protecting signal handling setup/teardown glock sync.RWMutex // global lock for all ops, sm validity opcount int32 // number of ops in flight + relonce sync.Once // once-er to ensure we only release once releasing int32 // flag indicating release of sm has begun - released int32 // flag indicating release of sm has finished } type smIsReleased struct{} @@ -171,9 +171,9 @@ func NewSourceManager(an ProjectAnalyzer, cachedir string) (*SourceMgr, error) { return sm, nil } -// SetUpSigHandling sets up typical os.Interrupt signal handling for a +// UseDefaultSignalHandling sets up typical os.Interrupt signal handling for a // SourceMgr. -func SetUpSigHandling(sm *SourceMgr) { +func (sm *SourceMgr) UseDefaultSignalHandling() { sigch := make(chan os.Signal, 1) signal.Notify(sigch, os.Interrupt) sm.HandleSignals(sigch) @@ -221,26 +221,19 @@ func (sm *SourceMgr) HandleSignals(sigch chan os.Signal) { return } - // Keep track of whether we waited for output purposes - var waited bool opc := sm.opcount if opc > 0 { - waited = true - fmt.Printf("Waiting for %v ops to complete...", opc) + fmt.Printf("Signal received: waiting for %v ops to complete...\n", opc) } // Mutex interaction in a signal handler is, as a general rule, // unsafe. I'm not clear on whether the guarantees Go provides // around signal handling, or having passed this through a // channel in general, obviate those concerns, but it's a lot - // easier to just hit the mutex right now, so do that until it - // proves problematic or someone provides a clear explanation. - sm.glock.Lock() - if waited && sm.released != 1 { - fmt.Print("done.\n") - } - sm.doRelease() - sm.glock.Unlock() + // easier to just rely on the mutex contained in the Once right + // now, so do that until it proves problematic or someone + // provides a clear explanation. + sm.relonce.Do(func() { sm.doRelease() }) return case <-qch: // quit channel triggered - deregister our sigch and return @@ -284,38 +277,38 @@ func (e CouldNotCreateLockError) Error() string { // longer safe to call methods against it; all method calls will immediately // result in errors. func (sm *SourceMgr) Release() { - // This ensures a signal handling can't interleave with a Release call - - // exit early if we're already marked as having initiated a release process. - // - // Setting it before we acquire the lock also guarantees that no _more_ - // method calls will stack up. - if !atomic.CompareAndSwapInt32(&sm.releasing, 0, 1) { - return - } + // Set sm.releasing before entering the Once func to guarantee that no + // _more_ method calls will stack up if/while waiting. + atomic.CompareAndSwapInt32(&sm.releasing, 0, 1) + + // Whether 'releasing' is set or not, we don't want this function to return + // until after the doRelease process is done, as doing so could cause the + // process to terminate before a signal-driven doRelease() call has a chance + // to finish its cleanup. + sm.relonce.Do(func() { sm.doRelease() }) +} +// doRelease actually releases physical resources (files on disk, etc.). +// +// This must be called only and exactly once. Calls to it should be wrapped in +// the sm.relonce sync.Once instance. +func (sm *SourceMgr) doRelease() { // Grab the global sm lock so that we only release once we're sure all other // calls have completed // // (This could deadlock, ofc) sm.glock.Lock() - sm.doRelease() - sm.glock.Unlock() -} -// doRelease actually releases physical resources (files on disk, etc.). -func (sm *SourceMgr) doRelease() { - // One last atomic marker ensures actual disk changes only happen once. - if atomic.CompareAndSwapInt32(&sm.released, 0, 1) { - // Close the file handle for the lock file - sm.lf.Close() - // Remove the lock file from disk - os.Remove(filepath.Join(sm.cachedir, "sm.lock")) - // Close the qch, if non-nil, so the signal handlers run out. This will - // also deregister the sig channel, if any has been set up. - if sm.qch != nil { - close(sm.qch) - } + // Close the file handle for the lock file + sm.lf.Close() + // Remove the lock file from disk + os.Remove(filepath.Join(sm.cachedir, "sm.lock")) + // Close the qch, if non-nil, so the signal handlers run out. This will + // also deregister the sig channel, if any has been set up. + if sm.qch != nil { + close(sm.qch) } + sm.glock.Unlock() } // AnalyzerInfo reports the name and version of the injected ProjectAnalyzer.