diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index e24afbf0d0..b8d7d5ac28 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -275,42 +275,22 @@ func (sm *SourceMgr) HandleSignals(sigch chan os.Signal) { // Run a new goroutine with the input sigch and the fresh qch go func(sch chan os.Signal, qch <-chan struct{}) { defer signal.Stop(sch) - for { - select { - case <-sch: - // Set up a timer to uninstall the signal handler after three - // seconds, so that the user can easily force termination with a - // second ctrl-c - go func(c <-chan time.Time) { - <-c - signal.Stop(sch) - }(time.After(3 * time.Second)) - - if !atomic.CompareAndSwapInt32(&sm.releasing, 0, 1) { - // Something's already called Release() on this sm, so we - // don't have to do anything, as we'd just be redoing - // that work. Instead, deregister and return. - return - } - - opc := sm.suprvsr.count() - if opc > 0 { - 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 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 - return + select { + case <-sch: + // Set up a timer to uninstall the signal handler after three + // seconds, so that the user can easily force termination with a + // second ctrl-c + time.AfterFunc(3*time.Second, func() { + signal.Stop(sch) + }) + + if opc := sm.suprvsr.count(); opc > 0 { + fmt.Printf("Signal received: waiting for %v ops to complete...\n", opc) } + + sm.Release() + case <-qch: + // quit channel triggered - deregister our sigch and return } }(sigch, sm.qch) // Try to ensure handler is blocked in for-select before releasing the mutex @@ -349,38 +329,26 @@ func (e CouldNotCreateLockError) Error() string { // longer safe to call methods against it; all method calls will immediately // result in errors. func (sm *SourceMgr) Release() { - // 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) + atomic.StoreInt32(&sm.releasing, 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(sm.doRelease) -} + sm.relonce.Do(func() { + // Send the signal to the supervisor to cancel all running calls. + sm.cancelAll() + sm.suprvsr.wait() -// 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() { - // Send the signal to the supervisor to cancel all running calls - sm.cancelAll() - sm.suprvsr.wait() - - // Close the source coordinator. - sm.srcCoord.close() - - // Close the file handle for the lock file and remove it from disk - sm.lf.Unlock() - 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 source coordinator. + sm.srcCoord.close() + + // Close the file handle for the lock file and remove it from disk + sm.lf.Unlock() + 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) + } + }) } // GetManifestAndLock returns manifest and lock information for the provided @@ -388,7 +356,7 @@ func (sm *SourceMgr) doRelease() { // manifest and lock is delegated to the provided ProjectAnalyzer's // DeriveManifestAndLock() method. func (sm *SourceMgr) GetManifestAndLock(id ProjectIdentifier, v Version, an ProjectAnalyzer) (Manifest, Lock, error) { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return nil, nil, smIsReleased{} } @@ -403,7 +371,7 @@ func (sm *SourceMgr) GetManifestAndLock(id ProjectIdentifier, v Version, an Proj // ListPackages parses the tree of the Go packages at and below the ProjectRoot // of the given ProjectIdentifier, at the given version. func (sm *SourceMgr) ListPackages(id ProjectIdentifier, v Version) (pkgtree.PackageTree, error) { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return pkgtree.PackageTree{}, smIsReleased{} } @@ -428,7 +396,7 @@ func (sm *SourceMgr) ListPackages(id ProjectIdentifier, v Version) (pkgtree.Pack // is not accessible (network outage, access issues, or the resource actually // went away), an error will be returned. func (sm *SourceMgr) ListVersions(id ProjectIdentifier) ([]PairedVersion, error) { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return nil, smIsReleased{} } @@ -444,7 +412,7 @@ func (sm *SourceMgr) ListVersions(id ProjectIdentifier) ([]PairedVersion, error) // RevisionPresentIn indicates whether the provided Revision is present in the given // repository. func (sm *SourceMgr) RevisionPresentIn(id ProjectIdentifier, r Revision) (bool, error) { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return false, smIsReleased{} } @@ -460,7 +428,7 @@ func (sm *SourceMgr) RevisionPresentIn(id ProjectIdentifier, r Revision) (bool, // SourceExists checks if a repository exists, either upstream or in the cache, // for the provided ProjectIdentifier. func (sm *SourceMgr) SourceExists(id ProjectIdentifier) (bool, error) { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return false, smIsReleased{} } @@ -478,7 +446,7 @@ func (sm *SourceMgr) SourceExists(id ProjectIdentifier) (bool, error) { // // The primary use case for this is prefetching. func (sm *SourceMgr) SyncSourceFor(id ProjectIdentifier) error { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return smIsReleased{} } @@ -493,7 +461,7 @@ func (sm *SourceMgr) SyncSourceFor(id ProjectIdentifier) error { // ExportProject writes out the tree of the provided ProjectIdentifier's // ProjectRoot, at the provided version, to the provided directory. func (sm *SourceMgr) ExportProject(id ProjectIdentifier, v Version, to string) error { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return smIsReleased{} } @@ -513,7 +481,7 @@ func (sm *SourceMgr) ExportProject(id ProjectIdentifier, v Version, to string) e // paths. (A special exception is written for gopkg.in to minimize network // activity, as its behavior is well-structured) func (sm *SourceMgr) DeduceProjectRoot(ip string) (ProjectRoot, error) { - if atomic.CompareAndSwapInt32(&sm.releasing, 1, 1) { + if atomic.LoadInt32(&sm.releasing) == 1 { return "", smIsReleased{} }