Merge pull request #9259 from dokku/9254-plugins-common-does-not-build-on-windows

Build plugins/common on windows

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
 )

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

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()
+}

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()
+}

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)