From e7dab76eb9a207cebfc400bbaa7610b05784b728 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Sun, 13 Oct 2024 17:22:04 +0300
Subject: [PATCH 1/2] Add lockutil test

Verify that the lock works for threads in the same process.

The test fails on windows:

    === RUN   TestWithDirLock
        lockutil_test.go:54: Expected one writer, got [writer 19 writer 0]

(Keeping as separate commit to make it easy to reproduce)

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/lockutil/lockutil_test.go | 56 +++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 pkg/lockutil/lockutil_test.go

diff --git a/pkg/lockutil/lockutil_test.go b/pkg/lockutil/lockutil_test.go
new file mode 100644
index 00000000000..210110c4dac
--- /dev/null
+++ b/pkg/lockutil/lockutil_test.go
@@ -0,0 +1,56 @@
+package lockutil
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const parallel = 20
+
+func TestWithDirLock(t *testing.T) {
+	dir := t.TempDir()
+	log := filepath.Join(dir, "log")
+
+	errc := make(chan error, 10)
+	for i := 0; i < parallel; i++ {
+		go func() {
+			err := WithDirLock(dir, func() error {
+				if _, err := os.Stat(log); err == nil {
+					return nil
+				} else if !errors.Is(err, os.ErrNotExist) {
+					return err
+				}
+				logFile, err := os.OpenFile(log, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o640)
+				if err != nil {
+					return err
+				}
+				defer logFile.Close()
+				if _, err := fmt.Fprintf(logFile, "writer %d\n", i); err != nil {
+					return err
+				}
+				return logFile.Close()
+			})
+			errc <- err
+		}()
+	}
+
+	for i := 0; i < parallel; i++ {
+		err := <-errc
+		if err != nil {
+			t.Error(err)
+		}
+	}
+
+	data, err := os.ReadFile(log)
+	if err != nil {
+		t.Fatal(err)
+	}
+	lines := strings.Split(strings.Trim(string(data), "\n"), "\n")
+	if len(lines) != 1 {
+		t.Errorf("Expected one writer, got %v", lines)
+	}
+}

From 3840a3b364e6ed0e3ba823f33e133cdac021197e Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Sun, 13 Oct 2024 18:01:46 +0300
Subject: [PATCH 2/2] Fix windows WithDirLock to exclusive lock

We use LOCKFILE_FAIL_IMMEDIATELY, creating a shared lock that fail
immediately if the lock cannot be acquired. This does not match the
behavior of the unix version and does not make sense.

Using now LOCKFILE_EXCLUSIVE_LOCK to acquire exclusive lock blocking
until the lock can be acquired.

Add constants for the flags and add comments for the argument names to
prevent future errors.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/lockutil/lockutil_windows.go | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/pkg/lockutil/lockutil_windows.go b/pkg/lockutil/lockutil_windows.go
index 31f31294604..f191559ab51 100644
--- a/pkg/lockutil/lockutil_windows.go
+++ b/pkg/lockutil/lockutil_windows.go
@@ -33,20 +33,37 @@ var (
 	procUnlockFileEx = modkernel32.NewProc("UnlockFileEx")
 )
 
+const (
+	// see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203(v=vs.85).aspx
+	LOCKFILE_EXCLUSIVE_LOCK   = 0x00000002
+	LOCKFILE_FAIL_IMMEDIATELY = 0x00000001
+)
+
 func WithDirLock(dir string, fn func() error) error {
 	dirFile, err := os.OpenFile(dir+".lock", os.O_CREATE, 0o644)
 	if err != nil {
 		return err
 	}
 	defer dirFile.Close()
-	// see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203(v=vs.85).aspx
-	// 1 lock immediately
-	if err := lockFileEx(syscall.Handle(dirFile.Fd()), 1, 0, 1, 0, &syscall.Overlapped{}); err != nil {
+	if err := lockFileEx(
+		syscall.Handle(dirFile.Fd()), // hFile
+		LOCKFILE_EXCLUSIVE_LOCK,      // dwFlags
+		0,                            // dwReserved
+		1,                            // nNumberOfBytesToLockLow
+		0,                            // nNumberOfBytesToLockHigh
+		&syscall.Overlapped{},        // lpOverlapped
+	); err != nil {
 		return fmt.Errorf("failed to lock %q: %w", dir, err)
 	}
 
 	defer func() {
-		if err := unlockFileEx(syscall.Handle(dirFile.Fd()), 0, 1, 0, &syscall.Overlapped{}); err != nil {
+		if err := unlockFileEx(
+			syscall.Handle(dirFile.Fd()), // hFile
+			0,                            // dwReserved
+			1,                            // nNumberOfBytesToLockLow
+			0,                            // nNumberOfBytesToLockHigh
+			&syscall.Overlapped{},        // lpOverlapped
+		); err != nil {
 			logrus.WithError(err).Errorf("failed to unlock %q", dir)
 		}
 	}()