UnlockAllObtained() method; FileStorage handles stale locks

It's still pretty early (day 2!) of the library so I'm OK with adding
a necessary method that removes locks that would become stale.

Also handle stale locks in the FileStorage implementation of Storage.

filestorage.go

@@ -17,6 +17,7 @@ package certmagic
 import (
 	"fmt"
 	"io/ioutil"
+	"log"
 	"os"
 	"path"
 	"path/filepath"
@@ -171,18 +172,36 @@ func (fs FileStorage) TryLock(key string) (Waiter, error) {
 	}
 
 	fw = &fileStorageWaiter{
+		key:      key,
 		filename: filepath.Join(lockDir, StorageKeys.safe(key)+".lock"),
 		wg:       new(sync.WaitGroup),
 	}
 
+	var checkedStaleLock bool // sentinel value to avoid infinite goto-ing
+
+createLock:
 	// create the file in a special mode such that an
 	// error is returned if it already exists
 	lf, err := os.OpenFile(fw.filename, os.O_CREATE|os.O_EXCL, 0644)
 	if err != nil {
 		if os.IsExist(err) {
-			// another process has the lock; use it to wait
+			// another process has the lock
+
+			// check to see if the lock is stale, if we haven't already
+			if !checkedStaleLock {
+				checkedStaleLock = true
+				if fs.lockFileStale(fw.filename) {
+					log.Printf("[INFO][%s] Lock for '%s' is stale; removing then retrying: %s",
+						fs, key, fw.filename)
+					os.Remove(fw.filename)
+					goto createLock
+				}
+			}
+
+			// if lock is not stale, wait upon it
 			return fw, nil
 		}
+
 		// otherwise, this was some unexpected error
 		return nil, err
 	}
@@ -225,6 +244,25 @@ func (fs FileStorage) Unlock(key string) error {
 	return nil
 }
 
+// UnlockAllObtained removes all locks obtained by
+// this instance of fs.
+func (fs FileStorage) UnlockAllObtained() {
+	for key, fw := range fileStorageNameLocks {
+		err := fs.Unlock(fw.key)
+		if err != nil {
+			log.Printf("[ERROR][%s] Releasing obtained lock for %s: %v", fs, key, err)
+		}
+	}
+}
+
+func (fs FileStorage) lockFileStale(filename string) bool {
+	info, err := os.Stat(filename)
+	if err != nil {
+		return true // no good way to handle this, really; lock is useless?
+	}
+	return time.Since(info.ModTime()) > staleLockDuration
+}
+
 func (fs FileStorage) lockDir() string {
 	return filepath.Join(fs.Path, "locks")
 }
@@ -241,6 +279,7 @@ func (fs FileStorage) String() string {
 // the lock will still block, but must wait for the
 // polling to get their answer.)
 type fileStorageWaiter struct {
+	key      string
 	filename string
 	wg       *sync.WaitGroup
 }
@@ -263,3 +302,7 @@ var fileStorageNameLocksMu sync.Mutex
 
 var _ Storage = FileStorage{}
 var _ Waiter = &fileStorageWaiter{}
+
+// staleLockDuration is the length of time
+// before considering a lock to be stale.
+const staleLockDuration = 2 * time.Hour

storage.go

@@ -30,6 +30,8 @@ import (
 // same Storage value (its implementation and configuration)
 // in order to share certificates and other TLS resources
 // with the cluster.
+//
+// Implementations of Storage must be safe for concurrent use.
 type Storage interface {
 	// Locker provides atomic synchronization
 	// operations, making Storage safe to share.
@@ -87,6 +89,15 @@ type Locker interface {
 	// TryLock or if Unlock was not called at all. Unlock should also
 	// clean up any unused resources allocated during TryLock.
 	Unlock(key string) error
+
+	// UnlockAllObtained removes all locks obtained by this process,
+	// upon which others may be waiting. The importer should call
+	// this on shutdowns (and crashes, ideally) to avoid leaving stale
+	// locks, but Locker implementations must NOT rely on this being
+	// the case and should anticipate and handle stale locks. Errors
+	// should be printed or logged, since there could be multiple,
+	// with no good way to handle them anyway.
+	UnlockAllObtained()
 }
 
 // Waiter is a type that can block until a lock is released.