fix(clientinject): Windows preflight no longer self-locks PE

OpenFile then CreateFile(share=0) always hit ERROR_SHARING_VIOLATION
in the same process, so Apply never ran on real Windows. Use path-only
exclusive CreateFile on Windows; keep flock-based probe on Unix.
This commit is contained in:
Reese Norris
2026-07-29 11:59:00 -04:00
parent 3e366183c9
commit 363fa54601
4 changed files with 50 additions and 32 deletions

View File

@@ -3,15 +3,15 @@ package clientinject
import (
"errors"
"fmt"
"os"
"strings"
)
// ErrClientRunning indicates the primary PE appears locked / in use.
var ErrClientRunning = errors.New("clientinject: client appears to be running (file locked); quit the client completely, then Apply again")
// PreflightPrimaryPE tries to open the primary PE exclusively (O_RDWR +
// platform exclusive lock / CreateFile share-mode 0 on Windows).
// PreflightPrimaryPE tries to open the primary PE exclusively.
// On Windows this is CreateFileW with dwShareMode=0 (no prior open handle).
// On Unix this is O_RDWR + flock(LOCK_EX|LOCK_NB).
// On lock/sharing failures it returns ErrClientRunning.
//
// This is a best-effort probe only: the exclusive lock is released before
@@ -22,22 +22,12 @@ func PreflightPrimaryPE(path string) error {
if path == "" {
return fmt.Errorf("clientinject: preflight: empty primary PE path")
}
f, err := os.OpenFile(path, os.O_RDWR, 0)
if err != nil {
if isLockError(err) {
return fmt.Errorf("%w: open %s: %v", ErrClientRunning, path, err)
}
return fmt.Errorf("clientinject: preflight open %s: %w", path, err)
}
defer f.Close()
if err := tryExclusiveLock(f); err != nil {
if err := platformPreflightExclusive(path); err != nil {
if isLockError(err) {
return fmt.Errorf("%w: lock %s: %v", ErrClientRunning, path, err)
}
return fmt.Errorf("clientinject: preflight lock %s: %w", path, err)
return fmt.Errorf("clientinject: preflight %s: %w", path, err)
}
_ = unlockFile(f)
return nil
}

View File

@@ -36,25 +36,28 @@ func TestPreflightPrimaryPE_Empty(t *testing.T) {
}
func TestPreflightPrimaryPE_Locked(t *testing.T) {
if runtime.GOOS == "windows" {
// Windows LockFileEx/share semantics differ; exercise best-effort path only.
t.Log("windows: lock contention test is best-effort; skipping strict assertion")
}
dir := t.TempDir()
path := filepath.Join(dir, "app.exe")
if err := os.WriteFile(path, []byte("MZ\x00\x00"), 0o644); err != nil {
t.Fatal(err)
}
// Hold exclusive flock in this process.
// Hold an open handle so exclusive re-open / flock fails.
holder, err := os.OpenFile(path, os.O_RDWR, 0)
if err != nil {
t.Fatal(err)
}
defer holder.Close()
if err := tryExclusiveLock(holder); err != nil {
t.Fatalf("hold lock: %v", err)
if runtime.GOOS != "windows" {
// Unix: need flock on the holder; open alone does not block another open.
if err := tryExclusiveLock(holder); err != nil {
t.Fatalf("hold lock: %v", err)
}
defer unlockFile(holder)
}
defer unlockFile(holder)
// Windows: os.OpenFile hold is enough — CreateFile(share=0) fails while any
// handle is open (including this process). Do not OpenFile+CreateFile exclusive
// in the same preflight path (that always self-conflicts).
err = PreflightPrimaryPE(path)
if err == nil {

View File

@@ -8,6 +8,25 @@ import (
"syscall"
)
// platformPreflightExclusive opens the path O_RDWR and takes a non-blocking
// exclusive flock, then releases both.
func platformPreflightExclusive(path string) error {
f, err := os.OpenFile(path, os.O_RDWR, 0)
if err != nil {
if isLockError(err) {
return fmt.Errorf("%w: open %s: %v", ErrClientRunning, path, err)
}
return fmt.Errorf("open %s: %w", path, err)
}
defer f.Close()
if err := tryExclusiveLock(f); err != nil {
return err
}
_ = unlockFile(f)
return nil
}
func tryExclusiveLock(f *os.File) error {
err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)
if err != nil {

View File

@@ -15,8 +15,10 @@ import (
// running EXEs usually do not hold range locks — LockFileEx can succeed while
// the client is still running.
//
// We always attempt CreateFile exclusive. LockFileEx is a secondary signal only
// (returns ErrClientRunning when it reports a lock/sharing violation).
// Important: do NOT open the path with os.OpenFile before CreateFile exclusive.
// A same-process open handle makes CreateFile(share=0) always fail with
// ERROR_SHARING_VIOLATION (confirmed on Windows 10/11). Path-based exclusive
// open is the only correct preflight pattern on Windows.
var (
modkernel32 = syscall.NewLazyDLL("kernel32.dll")
@@ -38,12 +40,17 @@ const (
_ERROR_SHARING_VIOLATION = 32
)
// platformPreflightExclusive is the Windows "client running" probe: CreateFile
// with share mode 0, then close. No long-lived handle.
func platformPreflightExclusive(path string) error {
return tryCreateFileExclusive(path)
}
// tryExclusiveLock is used by tests that already hold an *os.File.
// Prefer platformPreflightExclusive for production preflight.
// CreateFile exclusive is attempted only after the caller's handle would
// conflict — so this path uses LockFileEx on the existing handle only.
func tryExclusiveLock(f *os.File) error {
// Primary gate: CreateFile with share mode 0 (always).
if err := tryCreateFileExclusive(f.Name()); err != nil {
return err
}
// Secondary: LockFileEx on the already-open Go handle (best-effort).
var ol syscall.Overlapped
r1, _, e1 := procLockFileEx.Call(
f.Fd(),
@@ -59,8 +66,7 @@ func tryExclusiveLock(f *os.File) error {
errno == syscall.Errno(_ERROR_SHARING_VIOLATION) {
return fmt.Errorf("%w: LockFileEx: %v", ErrClientRunning, errno)
}
// Non-lock LockFileEx failure after CreateFile exclusive succeeded:
// treat as probe noise (CreateFile already proved exclusive open).
// Non-lock LockFileEx failure: treat as probe noise.
return nil
}
return nil