From b48e9e9189dd0e8083db9bd0378704524e670edb Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 18 Aug 2026 15:05:04 +0200 Subject: [PATCH] fix(windows): serialize claim operations --- tools/tht/internal/safeio/claim_windows.go | 12 ++++ .../tht/internal/safeio/claim_windows_test.go | 65 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/tools/tht/internal/safeio/claim_windows.go b/tools/tht/internal/safeio/claim_windows.go index 1173b6c5..7893341b 100644 --- a/tools/tht/internal/safeio/claim_windows.go +++ b/tools/tht/internal/safeio/claim_windows.go @@ -4,10 +4,16 @@ package safeio import ( "path/filepath" + "sync" "golang.org/x/sys/windows" ) +// Windows retained directory handles intentionally omit FILE_SHARE_DELETE. Serialize the +// complete path-based claim operations within this process so one operation cannot exhaust +// another's bounded external-contention retry while its validated parent handle is retained. +var windowsPrivateClaimOperationMu sync.Mutex + type windowsPrivateRegular struct { parents *windowsParentHandles handle windows.Handle @@ -48,6 +54,8 @@ func openWindowsPrivateRegular(path string, links uint32) (*windowsPrivateRegula } func claimCanonicalPrivateRegular(source, claim string) (claimed bool, resultErr error) { + windowsPrivateClaimOperationMu.Lock() + defer windowsPrivateClaimOperationMu.Unlock() directory, sourceName, claimName, err := openWindowsPrivateClaimDirectory(source, claim) if err != nil { return false, ErrUnsafeFile @@ -62,6 +70,8 @@ func claimCanonicalPrivateRegular(source, claim string) (claimed bool, resultErr } func readCanonicalPrivateClaim(source, claim string, maximum int64) (contents []byte, found bool, resultErr error) { + windowsPrivateClaimOperationMu.Lock() + defer windowsPrivateClaimOperationMu.Unlock() directory, sourceName, claimName, err := openWindowsPrivateClaimDirectory(source, claim) if err != nil { return nil, false, ErrUnsafeFile @@ -94,6 +104,8 @@ func openWindowsPrivateClaimDirectory(source, claim string) (PrivateDirectoryHan } func removeCanonicalPrivateClaim(source, claim string) (removed bool, resultErr error) { + windowsPrivateClaimOperationMu.Lock() + defer windowsPrivateClaimOperationMu.Unlock() directory, sourceName, claimName, err := openWindowsPrivateClaimDirectory(source, claim) if err != nil { return false, ErrUnsafeFile diff --git a/tools/tht/internal/safeio/claim_windows_test.go b/tools/tht/internal/safeio/claim_windows_test.go index 276f30c2..b5a65c2f 100644 --- a/tools/tht/internal/safeio/claim_windows_test.go +++ b/tools/tht/internal/safeio/claim_windows_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "sync" "testing" + "time" ) func createWindowsPrivateTestFile(t *testing.T, path string, contents []byte) { @@ -100,6 +101,70 @@ func TestRemoveCanonicalPrivateClaimPreservesOrphan(t *testing.T) { } } +func TestCanonicalPrivateClaimWaitsForRetainedRemoveOperation(t *testing.T) { + parent := filepath.Join(t.TempDir(), "claims") + if err := os.Mkdir(parent, 0o700); err != nil { + t.Fatal(err) + } + if err := ProtectPrivateDirectory(parent); err != nil { + t.Fatal(err) + } + source := filepath.Join(parent, "state.json") + claim := filepath.Join(parent, "state.claim") + createWindowsPrivateTestFile(t, source, []byte("state")) + if claimed, err := ClaimCanonicalPrivateRegular(source, claim); err != nil || !claimed { + t.Fatalf("ClaimCanonicalPrivateRegular() = claimed %v, err %v", claimed, err) + } + + removeOpened := make(chan struct{}) + releaseRemove := make(chan struct{}) + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(releaseRemove) }) } + defer release() + restoreHook := SetPrivateDirectoryTestHookForTest(func(stage string) { + if stage != "after-canonical-private-claim-parent-open" { + return + } + select { + case <-removeOpened: + default: + close(removeOpened) + } + <-releaseRemove + }) + defer restoreHook() + + type result struct { + changed bool + err error + } + removeResult := make(chan result, 1) + go func() { + removed, err := RemoveCanonicalPrivateClaim(source, claim) + removeResult <- result{changed: removed, err: err} + }() + <-removeOpened + + claimResult := make(chan result, 1) + go func() { + claimed, err := ClaimCanonicalPrivateRegular(source, claim) + claimResult <- result{changed: claimed, err: err} + }() + select { + case got := <-claimResult: + t.Fatalf("concurrent claim returned before retained removal completed: claimed %v, err %v", got.changed, got.err) + case <-time.After(250 * time.Millisecond): + } + + release() + if got := <-removeResult; got.err != nil || !got.changed { + t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, err %v", got.changed, got.err) + } + if got := <-claimResult; got.err != nil || got.changed { + t.Fatalf("concurrent ClaimCanonicalPrivateRegular() = claimed %v, err %v, want false/nil", got.changed, got.err) + } +} + func TestRemoveCanonicalPrivateClaimRejectsMismatchedTwoLinkFiles(t *testing.T) { parent := filepath.Join(t.TempDir(), "claims") if err := os.Mkdir(parent, 0o700); err != nil {