Merge branch 'master' of https://github.com/nightlyone/lockfile into …

…upstream-changes

lockfile.go

@@ -23,8 +23,9 @@ 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")
+//		fmt.Println("I am a temporary error situation, so wait and retry")
 //	}
 func (t TemporaryError) Temporary() bool { return true }
 
@@ -43,6 +44,7 @@ func New(path string) (Lockfile, error) {
 	if !filepath.IsAbs(path) {
 		return Lockfile(""), ErrNeedAbsPath
 	}
+
 	return Lockfile(path), nil
 }
 
@@ -61,6 +63,7 @@ func (l Lockfile) GetOwner() (*os.Process, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	running, err := isRunning(pid)
 	if err != nil {
 		return nil, err
@@ -71,10 +74,11 @@ func (l Lockfile) GetOwner() (*os.Process, error) {
 		if err != nil {
 			return nil, err
 		}
+
 		return proc, nil
 	}
-	return nil, ErrDeadOwner
 
+	return nil, ErrDeadOwner
 }
 
 // TryLock tries to own the lock.
@@ -91,34 +95,38 @@ func (l Lockfile) TryLock() error {
 		panic(ErrNeedAbsPath)
 	}
 
-	tmplock, err := ioutil.TempFile(filepath.Dir(name), "")
+	tmplock, cleanup, err := makePidFile(name, os.Getpid())
 	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
+	// EEXIST and similar error codes, caught by os.IsExist, are intentionally ignored,
+	// as it means that someone was faster creating this link
+	// and ignoring this kind of error is part of the algorithm.
+	// Then we will probably fail the pid owner check later, if this process is still alive.
+	// We cannot ignore ALL errors, since failure to support hard links, disk full
+	// as well as many other errors can happen to a filesystem operation
+	// and we really want to abort on those.
+	if err := os.Link(tmplock, name); err != nil {
+		if !os.IsExist(err) {
+			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())
+	fiTmp, err := os.Lstat(tmplock)
 	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
 	}
 
@@ -155,7 +163,7 @@ func (l Lockfile) TryLock() error {
 	return l.TryLock()
 }
 
-// Unlock a lock again, if we owned it. Returns any error that happend during release of lock.
+// Unlock a lock again, if we owned it. Returns any error that happened during release of lock.
 func (l Lockfile) Unlock() error {
 	proc, err := l.GetOwner()
 	switch err {
@@ -179,11 +187,6 @@ func (l Lockfile) Unlock() error {
 	}
 }
 
-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
@@ -197,5 +200,25 @@ func scanPidLine(content []byte) (int, error) {
 	if pid <= 0 {
 		return 0, ErrInvalidPid
 	}
+
 	return pid, nil
 }
+
+func makePidFile(name string, pid int) (tmpname string, cleanup func(), err error) {
+	tmplock, err := ioutil.TempFile(filepath.Dir(name), filepath.Base(name)+".")
+	if err != nil {
+		return "", nil, err
+	}
+
+	cleanup = func() {
+		_ = tmplock.Close()
+		_ = os.Remove(tmplock.Name())
+	}
+
+	if _, err := io.WriteString(tmplock, fmt.Sprintf("%d\n", pid)); err != nil {
+		cleanup() // Do cleanup here, so call doesn't have to.
+		return "", nil, err
+	}
+
+	return tmplock.Name(), cleanup, nil
+}

lockfile_test.go

@@ -16,15 +16,19 @@ func ExampleLockfile() {
 		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 {
+	if err = lock.TryLock(); err != nil {
 		fmt.Printf("Cannot lock %q, reason: %v", lock, err)
 		panic(err) // handle properly please!
 	}
 
-	defer lock.Unlock()
+	defer func() {
+		if err := lock.Unlock(); err != nil {
+			fmt.Printf("Cannot unlock %q, reason: %v", lock, err)
+			panic(err) // handle properly please!
+		}
+	}()
 
 	fmt.Println("Do stuff under lock")
 	// Output: Do stuff under lock
@@ -61,7 +65,6 @@ func TestBasicLockUnlock(t *testing.T) {
 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
@@ -71,7 +74,9 @@ func GetDeadPID() int {
 		if seen[pid] {
 			continue
 		}
+
 		seen[pid] = true
+
 		running, err := isRunning(pid)
 		if err != nil {
 			fmt.Println("Error checking PID: ", err)
@@ -164,6 +169,7 @@ func TestRogueDeletionDeadPid(t *testing.T) {
 		t.Fatal(err)
 		return
 	}
+
 	defer os.Remove(path)
 
 	err = lf.Unlock()
@@ -172,12 +178,12 @@ func TestRogueDeletionDeadPid(t *testing.T) {
 		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)
+	if _, err := os.Stat(path); err != nil {
+		if os.IsNotExist(err) {
+			content, _ := ioutil.ReadFile(path)
+			t.Fatalf("lockfile %q (%q) should not be deleted by us, if we didn't create it", path, content)
 		}
+		t.Fatalf("unexpected error %v", err)
 	}
 }
 
@@ -215,10 +221,12 @@ func TestInvalidPidLeadToReplacedLockfileAndSuccess(t *testing.T) {
 		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)
@@ -250,46 +258,27 @@ func TestScanPidLine(t *testing.T) {
 		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,
-		},
+		{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 {
+
+		if want := tc.pid; got != want {
 			t.Errorf("%d: expected pid %d, got %d", step, want, got)
 		}
 	}
@@ -299,9 +288,9 @@ func TestScanPidLine(t *testing.T) {
 		if tc.xfail == nil {
 			continue
 		}
-		want := tc.xfail
+
 		_, got := scanPidLine(tc.input)
-		if got != want {
+		if want := tc.xfail; got != want {
 			t.Errorf("%d: expected error %v, got %v", step, want, got)
 		}
 	}

lockfile_unix.go

@@ -1,4 +1,4 @@
-// +build darwin dragonfly freebsd linux nacl netbsd openbsd solaris
+// +build darwin dragonfly freebsd linux nacl netbsd openbsd solaris aix
 
 package lockfile
 
@@ -16,5 +16,6 @@ func isRunning(pid int) (bool, error) {
 	if err := proc.Signal(syscall.Signal(0)); err != nil {
 		return false, nil
 	}
+
 	return true, nil
 }