From 484563282bfba70200665ae48d576634931f4bde Mon Sep 17 00:00:00 2001 From: Nathan Ollerenshaw Date: Wed, 19 Jul 2017 00:03:15 -0700 Subject: [PATCH] Try to fix stale lock file problems. bug #820. --- Gopkg.lock | 8 ++++++- internal/gps/source_manager.go | 40 ++++++++++++++++++++++++---------- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 462c222384..67b2284ca4 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -25,6 +25,12 @@ packages = ["."] revision = "cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b" +[[projects]] + branch = "master" + name = "github.com/nightlyone/lockfile" + packages = ["."] + revision = "e83dc5e7bba095e8d32fb2124714bf41f2a30cb5" + [[projects]] name = "github.com/pelletier/go-buffruneio" packages = ["."] @@ -52,6 +58,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "1096dfb111cbe243aa24ea824ea3e1db7bb178c01d5565107c6d9290d225d722" + inputs-digest = "48b2f69d59b2f321b14ee11547f4ac94468ac91bb99aad48337d42b75b791975" solver-name = "gps-cdcl" solver-version = 1 diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 9eabb19354..8393a7fdb5 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -19,6 +19,7 @@ import ( "time" "github.com/golang/dep/internal/gps/pkgtree" + "github.com/nightlyone/lockfile" "github.com/pkg/errors" "github.com/sdboyer/constext" ) @@ -115,7 +116,7 @@ func (p ProjectAnalyzerInfo) String() string { // tools; control via dependency injection is intended to be sufficient. type SourceMgr struct { cachedir string // path to root of cache dir - lf *os.File // handle for the sm lock file on disk + lf *lockfile.Lockfile // handle for the sm lock file on disk suprvsr *supervisor // subsystem that supervises running calls/io cancelAll context.CancelFunc // cancel func to kill all running work deduceCoord *deductionCoordinator // subsystem that manages import path deduction @@ -153,21 +154,38 @@ func NewSourceManager(cachedir string) (*SourceMgr, error) { return nil, err } + // Fix for #820 + // + // Consult https://godoc.org/github.com/nightlyone/lockfile for the lockfile + // behaviour. It's magic. It deals with stale processes, and if there is + // a process keeping the lock busy, it will pass back a temporary error that + // we can spin on. + glpath := filepath.Join(cachedir, "sm.lock") - _, err = os.Stat(glpath) - if err == nil { + lockfile, err := lockfile.New(glpath) + if err != nil { return nil, CouldNotCreateLockError{ Path: glpath, - Err: fmt.Errorf("cache lock file %s exists - another process crashed or is still running?", glpath), + Err: fmt.Errorf("Unable to create lock %s: %s", glpath, err.Error()), } } - fi, err := os.OpenFile(glpath, os.O_CREATE|os.O_EXCL, 0600) // is 0600 sane for this purpose? - if err != nil { - return nil, CouldNotCreateLockError{ - Path: glpath, - Err: fmt.Errorf("err on attempting to create global cache lock: %s", err), + // If it's a TemporaryError, we retry every second. Otherwise, we fail + // permanently. + + err = lockfile.TryLock() + for err != nil { + if _, ok := err.(interface { + Temporary() bool + }); ok { + time.Sleep(time.Second * 1) + } else { + return nil, CouldNotCreateLockError{ + Path: glpath, + Err: fmt.Errorf("Unable to lock %s: %s", glpath, err.Error()), + } } + err = lockfile.TryLock() } ctx, cf := context.WithCancel(context.TODO()) @@ -176,7 +194,7 @@ func NewSourceManager(cachedir string) (*SourceMgr, error) { sm := &SourceMgr{ cachedir: cachedir, - lf: fi, + lf: &lockfile, suprvsr: superv, cancelAll: cf, deduceCoord: deducer, @@ -314,7 +332,7 @@ func (sm *SourceMgr) doRelease() { sm.suprvsr.wait() // Close the file handle for the lock file and remove it from disk - sm.lf.Close() + 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