From 06a6fffb596173379a4ab0d8bbd0612a6d5501cd Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 8 Nov 2026 21:13:25 -0500 Subject: [PATCH] fix: build plugins/common on windows On windows, `LockDirectory` now takes a `LockFileEx` lock on a `.dokku.lock` file inside the directory, as `flock` is unavailable there. --- plugins/common/go.mod | 2 +- plugins/common/lock.go | 27 +++++++----------------- plugins/common/lock_unix.go | 25 ++++++++++++++++++++++ plugins/common/lock_windows.go | 35 +++++++++++++++++++++++++++++++ plugins/common/properties_test.go | 22 +++++++++++++++++++ 5 files changed, 91 insertions(+), 20 deletions(-) create mode 100644 plugins/common/lock_unix.go create mode 100644 plugins/common/lock_windows.go diff --git a/plugins/common/go.mod b/plugins/common/go.mod index a82d6e046..e8b716443 100644 --- a/plugins/common/go.mod +++ b/plugins/common/go.mod @@ -13,6 +13,7 @@ require ( github.com/spf13/pflag v1.1.0 golang.org/x/crypto v0.58.0 golang.org/x/sync v0.24.0 + golang.org/x/sys v0.49.0 mvdan.cc/sh/v3 v3.14.1 ) @@ -28,6 +29,5 @@ require ( github.com/stretchr/testify v1.12.1 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/net v0.60.0 // indirect - golang.org/x/sys v0.49.0 // indirect golang.org/x/text v0.43.0 // indirect ) diff --git a/plugins/common/lock.go b/plugins/common/lock.go index a520af12f..f65a67f95 100644 --- a/plugins/common/lock.go +++ b/plugins/common/lock.go @@ -1,11 +1,9 @@ package common import ( - "fmt" "os" "path/filepath" "strings" - "syscall" ) // LockedDirectoriesEnvVar names the environment variable a caller holding a @@ -15,11 +13,13 @@ import ( // does not deadlock waiting on its own parent. const LockedDirectoriesEnvVar = "DOKKU_LOCKED_DIRECTORIES" -// LockDirectory holds an exclusive flock on dir for the duration of fn. The -// directory must already exist. The lock is advisory: it serializes callers -// that go through LockDirectory, and does not block plain reads. Locks are -// taken per open file description, so goroutines in a single process exclude -// each other as well as separate processes do. +// LockDirectory holds an exclusive lock on dir for the duration of fn. The +// directory must already exist. On unix the lock is a flock on the directory +// itself, and on windows it is a LockFileEx lock on a lock file inside the +// directory. The lock is advisory: it serializes callers that go through +// LockDirectory, and does not block plain reads. Locks are taken per open +// file, so goroutines in a single process exclude each other as well as +// separate processes do. // // A directory listed in DOKKU_LOCKED_DIRECTORIES is treated as already held by // a parent process, and fn runs without taking the lock again. LockDirectory is @@ -31,18 +31,7 @@ func LockDirectory(dir string, fn func() error) error { return fn() } - file, err := os.Open(dir) - if err != nil { - return fmt.Errorf("Unable to open %s for locking: %w", dir, err) - } - defer file.Close() - - if err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX); err != nil { - return fmt.Errorf("Unable to lock %s: %w", dir, err) - } - defer syscall.Flock(int(file.Fd()), syscall.LOCK_UN) - - return fn() + return lockDirectory(dir, fn) } // isDirectoryLockHeld reports whether dir is listed in DOKKU_LOCKED_DIRECTORIES diff --git a/plugins/common/lock_unix.go b/plugins/common/lock_unix.go new file mode 100644 index 000000000..d28bb922f --- /dev/null +++ b/plugins/common/lock_unix.go @@ -0,0 +1,25 @@ +//go:build !windows + +package common + +import ( + "fmt" + "os" + "syscall" +) + +// lockDirectory holds an exclusive flock on dir for the duration of fn +func lockDirectory(dir string, fn func() error) error { + file, err := os.Open(dir) + if err != nil { + return fmt.Errorf("Unable to open %s for locking: %w", dir, err) + } + defer file.Close() + + if err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX); err != nil { + return fmt.Errorf("Unable to lock %s: %w", dir, err) + } + defer syscall.Flock(int(file.Fd()), syscall.LOCK_UN) + + return fn() +} diff --git a/plugins/common/lock_windows.go b/plugins/common/lock_windows.go new file mode 100644 index 000000000..68155d52a --- /dev/null +++ b/plugins/common/lock_windows.go @@ -0,0 +1,35 @@ +//go:build windows + +package common + +import ( + "fmt" + "os" + "path/filepath" + + "golang.org/x/sys/windows" +) + +// directoryLockFile names the file inside a locked directory that holds the +// lock on windows, where a directory handle cannot be locked directly. The +// file is left in place after unlocking, as removing it would race other +// callers waiting on the lock. +const directoryLockFile = ".dokku.lock" + +// lockDirectory holds an exclusive LockFileEx lock on the lock file inside dir +// for the duration of fn +func lockDirectory(dir string, fn func() error) error { + file, err := os.OpenFile(filepath.Join(dir, directoryLockFile), os.O_RDWR|os.O_CREATE, 0600) + if err != nil { + return fmt.Errorf("Unable to open %s for locking: %w", dir, err) + } + defer file.Close() + + handle := windows.Handle(file.Fd()) + if err := windows.LockFileEx(handle, windows.LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &windows.Overlapped{}); err != nil { + return fmt.Errorf("Unable to lock %s: %w", dir, err) + } + defer windows.UnlockFileEx(handle, 0, 1, 0, &windows.Overlapped{}) + + return fn() +} diff --git a/plugins/common/properties_test.go b/plugins/common/properties_test.go index 2182c3235..429c730eb 100644 --- a/plugins/common/properties_test.go +++ b/plugins/common/properties_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strconv" "sync" + "sync/atomic" "testing" "time" @@ -269,6 +270,27 @@ func TestPropertyLockSerializesReadModifyWrite(t *testing.T) { Expect(PropertyGet(propertyTestPlugin, propertyTestApp, "counter")).To(Equal("50")) } +func TestLockDirectorySerializesConcurrentCallers(t *testing.T) { + RegisterTestingT(t) + t.Setenv(LockedDirectoriesEnvVar, "") + + dir := t.TempDir() + var active atomic.Int32 + var overlapped atomic.Bool + Expect(runConcurrently(20, func(i int) error { + return LockDirectory(dir, func() error { + if active.Add(1) > 1 { + overlapped.Store(true) + } + defer active.Add(-1) + time.Sleep(5 * time.Millisecond) + return nil + }) + })).To(Succeed()) + + Expect(overlapped.Load()).To(BeFalse()) +} + func TestLockDirectorySkipsDirectoriesHeldByParent(t *testing.T) { setupPropertyMapTest(t)