Lock now takes a context and should honor cancellation (#66)

* Lock now takes a context and should honor cancellation

This allows callers to give up if they can't obtain a lock in a certain
timeframe and for resources to be cleaned up, avoiding potential
resource leaks.

Breaking change for any Storage implementations, sorry about that. (It's
why we're not 1.0 yet.) I'll reach out to known implementations; it's a
simple change.

* Rename obtainLock to acquireLock to be less ambiguous

In our package, "obtain" has a more common meaning related to certs

config.go

@@ -388,7 +388,7 @@ func (cfg *Config) obtainWithIssuer(ctx context.Context, issuer Issuer, name str
 
 	// ensure idempotency of the obtain operation for this name
 	lockKey := cfg.lockKey("cert_acme", name)
-	err := obtainLock(cfg.Storage, lockKey)
+	err := acquireLock(ctx, cfg.Storage, lockKey)
 	if err != nil {
 		return err
 	}
@@ -474,7 +474,7 @@ func (cfg *Config) renewWithIssuer(ctx context.Context, issuer Issuer, name stri
 
 	// ensure idempotency of the renew operation for this name
 	lockKey := cfg.lockKey("cert_acme", name)
-	err := obtainLock(cfg.Storage, lockKey)
+	err := acquireLock(ctx, cfg.Storage, lockKey)
 	if err != nil {
 		return err
 	}

filestorage.go

@@ -15,6 +15,7 @@
 package certmagic
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -124,7 +125,7 @@ func (fs *FileStorage) Filename(key string) string {
 
 // Lock obtains a lock named by the given key. It blocks
 // until the lock can be obtained or an error is returned.
-func (fs *FileStorage) Lock(key string) error {
+func (fs *FileStorage) Lock(ctx context.Context, key string) error {
 	filename := fs.lockFilename(key)
 
 	for {
@@ -168,8 +169,13 @@ func (fs *FileStorage) Lock(key string) error {
 
 		default:
 			// lockfile exists and is not stale;
-			// just wait a moment and try again
-			time.Sleep(fileLockPollInterval)
+			// just wait a moment and try again,
+			// or return if context cancelled
+			select {
+			case <-time.After(fileLockPollInterval):
+			case <-ctx.Done():
+				return ctx.Err()
+			}
 		}
 	}
 }

storage.go

@@ -15,6 +15,7 @@
 package certmagic
 
 import (
+	"context"
 	"log"
 	"path"
 	"regexp"
@@ -86,8 +87,11 @@ type Locker interface {
 	// To prevent deadlocks, all implementations (where this concern
 	// is relevant) should put a reasonable expiration on the lock in
 	// case Unlock is unable to be called due to some sort of network
-	// failure or system crash.
-	Lock(key string) error
+	// failure or system crash. Additionally, implementations should
+	// honor context cancellation as much as possible (in case the
+	// caller wishes to give up and free resources before the lock
+	// can be obtained).
+	Lock(ctx context.Context, key string) error
 
 	// Unlock releases the lock for key. This method must ONLY be
 	// called after a successful call to Lock, and only after the
@@ -223,8 +227,8 @@ func CleanUpOwnLocks() {
 	}
 }
 
-func obtainLock(storage Storage, lockKey string) error {
-	err := storage.Lock(lockKey)
+func acquireLock(ctx context.Context, storage Storage, lockKey string) error {
+	err := storage.Lock(ctx, lockKey)
 	if err == nil {
 		locksMu.Lock()
 		locks[lockKey] = storage