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/manager_test.go b/internal/gps/manager_test.go index d505846677..b5c15794a8 100644 --- a/internal/gps/manager_test.go +++ b/internal/gps/manager_test.go @@ -713,10 +713,6 @@ func TestSignalHandling(t *testing.T) { t.Error("Releasing flag did not get set") } - lpath := filepath.Join(sm.cachedir, "sm.lock") - if _, err := os.Stat(lpath); err == nil { - t.Fatal("Expected error on statting what should be an absent lock file") - } clean() // Test again, this time with a running call diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 9eabb19354..615cdb0cd0 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,56 @@ 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), + process, err := lockfile.GetOwner() + if err == nil { + // If we didn't get an error, then the lockfile exists already. We should + // check to see if it's us already: + if process.Pid == os.Getpid() { + return nil, CouldNotCreateLockError{ + Path: glpath, + Err: fmt.Errorf("lockfile %s already locked by this process", glpath), + } + } + + // There is a lockfile, but it's owned by someone else. We'll try to lock + // it anyway. + } + + // If it's a TemporaryError, we retry every second. Otherwise, we fail + // permanently. + // + // TODO: After some time, we should emit some kind of warning that we're waiting + // for the lockfile to be released. #534 should be address before we will do that. + + 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 +212,7 @@ func NewSourceManager(cachedir string) (*SourceMgr, error) { sm := &SourceMgr{ cachedir: cachedir, - lf: fi, + lf: &lockfile, suprvsr: superv, cancelAll: cf, deduceCoord: deducer, @@ -314,7 +350,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 diff --git a/vendor/github.com/nightlyone/lockfile/.gitignore b/vendor/github.com/nightlyone/lockfile/.gitignore new file mode 100644 index 0000000000..5a05665dee --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/.gitignore @@ -0,0 +1,27 @@ +# Compiled Object files, Static and Dynamic libs (Shared Objects) +*.o +*.a +*.so + +# Folders +_obj +_test + +# popular temporaries +.err +.out +.diff + +# Architecture specific extensions/prefixes +*.[568vq] +[568vq].out + +*.cgo1.go +*.cgo2.c +_cgo_defun.c +_cgo_gotypes.go +_cgo_export.* + +_testmain.go + +*.exe diff --git a/vendor/github.com/nightlyone/lockfile/.gitmodules b/vendor/github.com/nightlyone/lockfile/.gitmodules new file mode 100644 index 0000000000..6faa9e3469 --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/.gitmodules @@ -0,0 +1,3 @@ +[submodule "git-hooks"] + path = git-hooks + url = https://github.com/nightlyone/git-hooks diff --git a/vendor/github.com/nightlyone/lockfile/.travis.yml b/vendor/github.com/nightlyone/lockfile/.travis.yml new file mode 100644 index 0000000000..76e5962bf1 --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/.travis.yml @@ -0,0 +1,14 @@ +language: go +go: + - 1.4.3 + - 1.6.2 + - tip + +# Only test commits to production branch and all pull requests +branches: + only: + - master + +matrix: + allow_failures: + - go: tip diff --git a/vendor/github.com/nightlyone/lockfile/LICENSE b/vendor/github.com/nightlyone/lockfile/LICENSE new file mode 100644 index 0000000000..eb5b804685 --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/LICENSE @@ -0,0 +1,19 @@ +Copyright (c) 2012 Ingo Oeser + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/vendor/github.com/nightlyone/lockfile/README.md b/vendor/github.com/nightlyone/lockfile/README.md new file mode 100644 index 0000000000..c35235cdae --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/README.md @@ -0,0 +1,52 @@ +lockfile +========= +Handle locking via pid files. + +[![Build Status Unix][1]][2] +[![Build status Windows][3]][4] + +[1]: https://secure.travis-ci.org/nightlyone/lockfile.png +[2]: https://travis-ci.org/nightlyone/lockfile +[3]: https://ci.appveyor.com/api/projects/status/7mojkmauj81uvp8u/branch/master?svg=true +[4]: https://ci.appveyor.com/project/nightlyone/lockfile/branch/master + + + +install +------- +Install [Go 1][5], either [from source][6] or [with a prepackaged binary][7]. +For Windows suport, Go 1.4 or newer is required. + +Then run + + go get github.com/nightlyone/lockfile + +[5]: http://golang.org +[6]: http://golang.org/doc/install/source +[7]: http://golang.org/doc/install + +LICENSE +------- +MIT + +documentation +------------- +[package documentation at godoc.org](http://godoc.org/github.com/nightlyone/lockfile) + +install +------------------- + go get github.com/nightlyone/lockfile + + +contributing +============ + +Contributions are welcome. Please open an issue or send me a pull request for a dedicated branch. +Make sure the git commit hooks show it works. + +git commit hooks +----------------------- +enable commit hooks via + + cd .git ; rm -rf hooks; ln -s ../git-hooks hooks ; cd .. + diff --git a/vendor/github.com/nightlyone/lockfile/appveyor.yml b/vendor/github.com/nightlyone/lockfile/appveyor.yml new file mode 100644 index 0000000000..cf72a58b13 --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/appveyor.yml @@ -0,0 +1,12 @@ +clone_folder: c:\gopath\src\github.com\nightlyone\lockfile + +environment: + GOPATH: c:\gopath + +install: + - go version + - go env + - go get -v -t ./... + +build_script: + - go test -v ./... diff --git a/vendor/github.com/nightlyone/lockfile/lockfile.go b/vendor/github.com/nightlyone/lockfile/lockfile.go new file mode 100644 index 0000000000..2d956e07e3 --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/lockfile.go @@ -0,0 +1,201 @@ +// Package lockfile handles pid file based locking. +// While a sync.Mutex helps against concurrency issues within a single process, +// this package is designed to help against concurrency issues between cooperating processes +// or serializing multiple invocations of the same process. You can also combine sync.Mutex +// with Lockfile in order to serialize an action between different goroutines in a single program +// and also multiple invocations of this program. +package lockfile + +import ( + "errors" + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" +) + +// Lockfile is a pid file which can be locked +type Lockfile string + +// TemporaryError is a type of error where a retry after a random amount of sleep should help to mitigate it. +type TemporaryError string + +func (t TemporaryError) Error() string { return string(t) } + +// Temporary returns always true. +// It exists, so you can detect it via +// if te, ok := err.(interface{ Temporary() bool }); ok { +// fmt.Println("I am a temporay error situation, so wait and retry") +// } +func (t TemporaryError) Temporary() bool { return true } + +// Various errors returned by this package +var ( + ErrBusy = TemporaryError("Locked by other process") // If you get this, retry after a short sleep might help + ErrNotExist = TemporaryError("Lockfile created, but doesn't exist") // If you get this, retry after a short sleep might help + ErrNeedAbsPath = errors.New("Lockfiles must be given as absolute path names") + ErrInvalidPid = errors.New("Lockfile contains invalid pid for system") + ErrDeadOwner = errors.New("Lockfile contains pid of process not existent on this system anymore") + ErrRogueDeletion = errors.New("Lockfile owned by me has been removed unexpectedly") +) + +// New describes a new filename located at the given absolute path. +func New(path string) (Lockfile, error) { + if !filepath.IsAbs(path) { + return Lockfile(""), ErrNeedAbsPath + } + return Lockfile(path), nil +} + +// GetOwner returns who owns the lockfile. +func (l Lockfile) GetOwner() (*os.Process, error) { + name := string(l) + + // Ok, see, if we have a stale lockfile here + content, err := ioutil.ReadFile(name) + if err != nil { + return nil, err + } + + // try hard for pids. If no pid, the lockfile is junk anyway and we delete it. + pid, err := scanPidLine(content) + if err != nil { + return nil, err + } + running, err := isRunning(pid) + if err != nil { + return nil, err + } + + if running { + proc, err := os.FindProcess(pid) + if err != nil { + return nil, err + } + return proc, nil + } + return nil, ErrDeadOwner + +} + +// TryLock tries to own the lock. +// It Returns nil, if successful and and error describing the reason, it didn't work out. +// Please note, that existing lockfiles containing pids of dead processes +// and lockfiles containing no pid at all are simply deleted. +func (l Lockfile) TryLock() error { + name := string(l) + + // This has been checked by New already. If we trigger here, + // the caller didn't use New and re-implemented it's functionality badly. + // So panic, that he might find this easily during testing. + if !filepath.IsAbs(name) { + panic(ErrNeedAbsPath) + } + + tmplock, err := ioutil.TempFile(filepath.Dir(name), filepath.Base(name)+".") + if err != nil { + return err + } + + cleanup := func() { + _ = tmplock.Close() + _ = os.Remove(tmplock.Name()) + } + defer cleanup() + + if err := writePidLine(tmplock, os.Getpid()); err != nil { + return err + } + + // return value intentionally ignored, as ignoring it is part of the algorithm + _ = os.Link(tmplock.Name(), name) + + fiTmp, err := os.Lstat(tmplock.Name()) + if err != nil { + return err + } + fiLock, err := os.Lstat(name) + if err != nil { + // tell user that a retry would be a good idea + if os.IsNotExist(err) { + return ErrNotExist + } + return err + } + + // Success + if os.SameFile(fiTmp, fiLock) { + return nil + } + + proc, err := l.GetOwner() + switch err { + default: + // Other errors -> defensively fail and let caller handle this + return err + case nil: + if proc.Pid != os.Getpid() { + return ErrBusy + } + case ErrDeadOwner, ErrInvalidPid: + // cases we can fix below + } + + // clean stale/invalid lockfile + err = os.Remove(name) + if err != nil { + // If it doesn't exist, then it doesn't matter who removed it. + if !os.IsNotExist(err) { + return err + } + } + + // now that the stale lockfile is gone, let's recurse + return l.TryLock() +} + +// Unlock a lock again, if we owned it. Returns any error that happend during release of lock. +func (l Lockfile) Unlock() error { + proc, err := l.GetOwner() + switch err { + case ErrInvalidPid, ErrDeadOwner: + return ErrRogueDeletion + case nil: + if proc.Pid == os.Getpid() { + // we really own it, so let's remove it. + return os.Remove(string(l)) + } + // Not owned by me, so don't delete it. + return ErrRogueDeletion + default: + // This is an application error or system error. + // So give a better error for logging here. + if os.IsNotExist(err) { + return ErrRogueDeletion + } + // Other errors -> defensively fail and let caller handle this + return err + } +} + +func writePidLine(w io.Writer, pid int) error { + _, err := io.WriteString(w, fmt.Sprintf("%d\n", pid)) + return err +} + +func scanPidLine(content []byte) (int, error) { + if len(content) == 0 { + return 0, ErrInvalidPid + } + + var pid int + if _, err := fmt.Sscanln(string(content), &pid); err != nil { + return 0, ErrInvalidPid + } + + if pid <= 0 { + return 0, ErrInvalidPid + } + return pid, nil +} diff --git a/vendor/github.com/nightlyone/lockfile/lockfile_test.go b/vendor/github.com/nightlyone/lockfile/lockfile_test.go new file mode 100644 index 0000000000..be2c821e21 --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/lockfile_test.go @@ -0,0 +1,308 @@ +package lockfile + +import ( + "fmt" + "io/ioutil" + "math/rand" + "os" + "path/filepath" + "strconv" + "testing" +) + +func ExampleLockfile() { + lock, err := New(filepath.Join(os.TempDir(), "lock.me.now.lck")) + if err != nil { + fmt.Printf("Cannot init lock. reason: %v", err) + panic(err) // handle properly please! + } + err = lock.TryLock() + + // Error handling is essential, as we only try to get the lock. + if err != nil { + fmt.Printf("Cannot lock %q, reason: %v", lock, err) + panic(err) // handle properly please! + } + + defer lock.Unlock() + + fmt.Println("Do stuff under lock") + // Output: Do stuff under lock +} + +func TestBasicLockUnlock(t *testing.T) { + path, err := filepath.Abs("test_lockfile.pid") + if err != nil { + panic(err) + } + + lf, err := New(path) + if err != nil { + t.Fail() + fmt.Println("Error making lockfile: ", err) + return + } + + err = lf.TryLock() + if err != nil { + t.Fail() + fmt.Println("Error locking lockfile: ", err) + return + } + + err = lf.Unlock() + if err != nil { + t.Fail() + fmt.Println("Error unlocking lockfile: ", err) + return + } +} + +func GetDeadPID() int { + // I have no idea how windows handles large PIDs, or if they even exist. + // So limit it to be less or equal to 4096 to be safe. + + const maxPid = 4095 + + // limited iteration, so we finish one day + seen := map[int]bool{} + for len(seen) < maxPid { + pid := rand.Intn(maxPid + 1) // see https://godoc.org/math/rand#Intn why + if seen[pid] { + continue + } + seen[pid] = true + running, err := isRunning(pid) + if err != nil { + fmt.Println("Error checking PID: ", err) + continue + } + + if !running { + return pid + } + } + panic(fmt.Sprintf("all pids lower %d are used, cannot test this", maxPid)) +} + +func TestBusy(t *testing.T) { + path, err := filepath.Abs("test_lockfile.pid") + if err != nil { + t.Fatal(err) + return + } + + pid := os.Getppid() + + if err := ioutil.WriteFile(path, []byte(strconv.Itoa(pid)+"\n"), 0666); err != nil { + t.Fatal(err) + return + } + defer os.Remove(path) + + lf, err := New(path) + if err != nil { + t.Fatal(err) + return + } + + got := lf.TryLock() + if got != ErrBusy { + t.Fatalf("expected error %q, got %v", ErrBusy, got) + return + } +} + +func TestRogueDeletion(t *testing.T) { + path, err := filepath.Abs("test_lockfile.pid") + if err != nil { + t.Fatal(err) + return + } + lf, err := New(path) + if err != nil { + t.Fatal(err) + return + } + err = lf.TryLock() + if err != nil { + t.Fatal(err) + return + } + err = os.Remove(path) + if err != nil { + t.Fatal(err) + return + } + + got := lf.Unlock() + if got != ErrRogueDeletion { + t.Fatalf("unexpected error: %v", got) + return + } +} + +func TestRogueDeletionDeadPid(t *testing.T) { + path, err := filepath.Abs("test_lockfile.pid") + if err != nil { + t.Fatal(err) + return + } + lf, err := New(path) + if err != nil { + t.Fatal(err) + return + } + err = lf.TryLock() + if err != nil { + t.Fatal(err) + return + } + + pid := GetDeadPID() + if err := ioutil.WriteFile(path, []byte(strconv.Itoa(pid)+"\n"), 0666); err != nil { + t.Fatal(err) + return + } + defer os.Remove(path) + + err = lf.Unlock() + if err != ErrRogueDeletion { + t.Fatalf("unexpected error: %v", err) + return + } + + if _, err := os.Stat(path); os.IsNotExist(err) { + t.Fatal("lockfile should not be deleted by us, if we didn't create it") + } else { + if err != nil { + t.Fatalf("unexpected error %v", err) + } + } +} + +func TestRemovesStaleLockOnDeadOwner(t *testing.T) { + path, err := filepath.Abs("test_lockfile.pid") + if err != nil { + t.Fatal(err) + return + } + lf, err := New(path) + if err != nil { + t.Fatal(err) + return + } + pid := GetDeadPID() + if err := ioutil.WriteFile(path, []byte(strconv.Itoa(pid)+"\n"), 0666); err != nil { + t.Fatal(err) + return + } + err = lf.TryLock() + if err != nil { + t.Fatal(err) + return + } + + if err := lf.Unlock(); err != nil { + t.Fatal(err) + return + } +} + +func TestInvalidPidLeadToReplacedLockfileAndSuccess(t *testing.T) { + path, err := filepath.Abs("test_lockfile.pid") + if err != nil { + t.Fatal(err) + return + } + if err := ioutil.WriteFile(path, []byte("\n"), 0666); err != nil { + t.Fatal(err) + return + } + defer os.Remove(path) + + lf, err := New(path) + if err != nil { + t.Fatal(err) + return + } + + if err := lf.TryLock(); err != nil { + t.Fatalf("unexpected error: %v", err) + return + } + + // now check if file exists and contains the correct content + got, err := ioutil.ReadFile(path) + if err != nil { + t.Fatalf("unexpected error %v", err) + return + } + want := fmt.Sprintf("%d\n", os.Getpid()) + if string(got) != want { + t.Fatalf("got %q, want %q", got, want) + } +} + +func TestScanPidLine(t *testing.T) { + tests := [...]struct { + input []byte + pid int + xfail error + }{ + { + xfail: ErrInvalidPid, + }, + { + input: []byte(""), + xfail: ErrInvalidPid, + }, + { + input: []byte("\n"), + xfail: ErrInvalidPid, + }, + { + input: []byte("-1\n"), + xfail: ErrInvalidPid, + }, + { + input: []byte("0\n"), + xfail: ErrInvalidPid, + }, + { + input: []byte("a\n"), + xfail: ErrInvalidPid, + }, + { + input: []byte("1\n"), + pid: 1, + }, + } + + // test positive cases first + for step, tc := range tests { + if tc.xfail != nil { + continue + } + want := tc.pid + got, err := scanPidLine(tc.input) + if err != nil { + t.Fatalf("%d: unexpected error %v", step, err) + } + if got != want { + t.Errorf("%d: expected pid %d, got %d", step, want, got) + } + } + + // test negative cases now + for step, tc := range tests { + if tc.xfail == nil { + continue + } + want := tc.xfail + _, got := scanPidLine(tc.input) + if got != want { + t.Errorf("%d: expected error %v, got %v", step, want, got) + } + } +} diff --git a/vendor/github.com/nightlyone/lockfile/lockfile_unix.go b/vendor/github.com/nightlyone/lockfile/lockfile_unix.go new file mode 100644 index 0000000000..742b041fb6 --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/lockfile_unix.go @@ -0,0 +1,20 @@ +// +build darwin dragonfly freebsd linux nacl netbsd openbsd solaris + +package lockfile + +import ( + "os" + "syscall" +) + +func isRunning(pid int) (bool, error) { + proc, err := os.FindProcess(pid) + if err != nil { + return false, err + } + + if err := proc.Signal(syscall.Signal(0)); err != nil { + return false, nil + } + return true, nil +} diff --git a/vendor/github.com/nightlyone/lockfile/lockfile_windows.go b/vendor/github.com/nightlyone/lockfile/lockfile_windows.go new file mode 100644 index 0000000000..482bd91d7b --- /dev/null +++ b/vendor/github.com/nightlyone/lockfile/lockfile_windows.go @@ -0,0 +1,30 @@ +package lockfile + +import ( + "syscall" +) + +//For some reason these consts don't exist in syscall. +const ( + error_invalid_parameter = 87 + code_still_active = 259 +) + +func isRunning(pid int) (bool, error) { + procHnd, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION, true, uint32(pid)) + if err != nil { + if scerr, ok := err.(syscall.Errno); ok { + if uintptr(scerr) == error_invalid_parameter { + return false, nil + } + } + } + + var code uint32 + err = syscall.GetExitCodeProcess(procHnd, &code) + if err != nil { + return false, err + } + + return code == code_still_active, nil +}