Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lock now takes a context and should honor cancellation #66

Merged
merged 2 commits into from May 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions config.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 9 additions & 3 deletions filestorage.go
Expand Up @@ -15,6 +15,7 @@
package certmagic

import (
"context"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
}
}
}
Expand Down
12 changes: 8 additions & 4 deletions storage.go
Expand Up @@ -15,6 +15,7 @@
package certmagic

import (
"context"
"log"
"path"
"regexp"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down