amadan

carlos / platform Public

moves-held-lock

Squashed from moves-held-lock into main.
claude dde859f38182f5915706b0e3e8b17c2deea22019 parent 359e8ac
4 files changed, +570 −20
  • internal/host/movesfloor_linux.go +154 −16
  • internal/host/movesfloor_other.go +2 −0
  • internal/host/movesheldlock.go +23 −4
  • internal/host/movesheldlock_cleanup_test.go +391 −0

CI passed — run details

CI log 
=== gate ===
cd launchctx && go vet ./... && go test ./... -count=1
ok  	amadan.net/carlos/platform.git/launchctx	0.012s
?   	amadan.net/carlos/platform.git/launchctx/controlname	[no test files]
?   	amadan.net/carlos/platform.git/launchctx/wireid	[no test files]
go vet ./...
gofmt -l .: clean
go test ./... -count=1 -timeout 30m
ok  	github.com/carlosframework/platform	330.721s
?   	github.com/carlosframework/platform/cmd/changelog	[no test files]
ok  	github.com/carlosframework/platform/cmd/console	3.372s
ok  	github.com/carlosframework/platform/cmd/moveprobe	92.241s
ok  	github.com/carlosframework/platform/internal/activator	209.885s
ok  	github.com/carlosframework/platform/internal/agentsock	0.019s
ok  	github.com/carlosframework/platform/internal/alerting	0.003s
ok  	github.com/carlosframework/platform/internal/analytics	0.021s
ok  	github.com/carlosframework/platform/internal/appconfig	2.324s
ok  	github.com/carlosframework/platform/internal/apperr	0.077s
ok  	github.com/carlosframework/platform/internal/applog	0.137s
ok  	github.com/carlosframework/platform/internal/awsinv	0.004s
ok  	github.com/carlosframework/platform/internal/boxchoice	0.004s
ok  	github.com/carlosframework/platform/internal/boxid	0.922s
?   	github.com/carlosframework/platform/internal/boxid/boxidtest	[no test files]
?   	github.com/carlosframework/platform/internal/boxid/internal/signtest	[no test files]
ok  	github.com/carlosframework/platform/internal/boxvolumes	0.009s
ok  	github.com/carlosframework/platform/internal/caps	0.012s
ok  	github.com/carlosframework/platform/internal/certwarm	0.002s
ok  	github.com/carlosframework/platform/internal/cgroup	0.192s
ok  	github.com/carlosframework/platform/internal/changelog	0.265s
ok  	github.com/carlosframework/platform/internal/clamd	0.055s
?   	github.com/carlosframework/platform/internal/cliflag	[no test files]
ok  	github.com/carlosframework/platform/internal/console	355.023s
?   	github.com/carlosframework/platform/internal/controlname	[no test files]
?   	github.com/carlosframework/platform/internal/controlsock	[no test files]
ok  	github.com/carlosframework/platform/internal/dbdoor	5.680s
ok  	github.com/carlosframework/platform/internal/dbdoor/dbdoortest	0.019s
ok  	github.com/carlosframework/platform/internal/dbverify	0.004s
ok  	github.com/carlosframework/platform/internal/dnsrec	0.003s
ok  	github.com/carlosframework/platform/internal/docsite	0.018s
ok  	github.com/carlosframework/platform/internal/domains	2.025s
ok  	github.com/carlosframework/platform/internal/doorkey	0.006s
ok  	github.com/carlosframework/platform/internal/economics	0.005s
ok  	github.com/carlosframework/platform/internal/edge	25.912s
?   	github.com/carlosframework/platform/internal/edgeproof	[no test files]
ok  	github.com/carlosframework/platform/internal/edgesfile	0.007s
ok  	github.com/carlosframework/platform/internal/fleetapi	0.129s
ok  	github.com/carlosframework/platform/internal/gate	1.663s
ok  	github.com/carlosframework/platform/internal/genselect	0.019s
ok  	github.com/carlosframework/platform/internal/geo	0.030s
ok  	github.com/carlosframework/platform/internal/geoip	0.041s
ok  	github.com/carlosframework/platform/internal/host	52.749s
ok  	github.com/carlosframework/platform/internal/hostactors	0.400s
ok  	github.com/carlosframework/platform/internal/hostbox	1.294s
ok  	github.com/carlosframework/platform/internal/hostclaim	0.243s
ok  	github.com/carlosframework/platform/internal/hostevidence	0.006s
ok  	github.com/carlosframework/platform/internal/hostexec	0.302s
ok  	github.com/carlosframework/platform/internal/hostfence	0.045s
ok  	github.com/carlosframework/platform/internal/hostfiles	4.168s
ok  	github.com/carlosframework/platform/internal/hostfloor	0.017s
ok  	github.com/carlosframework/platform/internal/hostguard	0.139s
ok  	github.com/carlosframework/platform/internal/hostops	1.311s
?   	github.com/carlosframework/platform/internal/hostops/fencerelease	[no test files]
ok  	github.com/carlosframework/platform/internal/hostops/internal/fencerelease	0.005s
ok  	github.com/carlosframework/platform/internal/hostquarantine	0.004s
ok  	github.com/carlosframework/platform/internal/hostresolve	0.144s
ok  	github.com/carlosframework/platform/internal/hostwrite	0.002s
ok  	github.com/carlosframework/platform/internal/infraview	0.002s
ok  	github.com/carlosframework/platform/internal/instancectl	1.703s
ok  	github.com/carlosframework/platform/internal/instanceexec	2.692s
ok  	github.com/carlosframework/platform/internal/instancefs	0.012s
ok  	github.com/carlosframework/platform/internal/instancekey	0.012s
ok  	github.com/carlosframework/platform/internal/instanceupdates	0.012s
ok  	github.com/carlosframework/platform/internal/layout	1.344s
ok  	github.com/carlosframework/platform/internal/lease	9.441s
ok  	github.com/carlosframework/platform/internal/lease/leasetest	0.011s
ok  	github.com/carlosframework/platform/internal/ledger	0.012s
ok  	github.com/carlosframework/platform/internal/mail	0.160s
ok  	github.com/carlosframework/platform/internal/maintenance	0.077s
ok  	github.com/carlosframework/platform/internal/maintenanceexec	5.504s
ok  	github.com/carlosframework/platform/internal/maintenanceguard	0.067s
ok  	github.com/carlosframework/platform/internal/metrics	0.008s
ok  	github.com/carlosframework/platform/internal/moves	21.095s
ok  	github.com/carlosframework/platform/internal/moves/archive	0.725s
ok  	github.com/carlosframework/platform/internal/objstore	0.163s
ok  	github.com/carlosframework/platform/internal/ops	0.153s
ok  	github.com/carlosframework/platform/internal/park	0.182s
ok  	github.com/carlosframework/platform/internal/privatefile	0.004s
ok  	github.com/carlosframework/platform/internal/provision	3.171s
ok  	github.com/carlosframework/platform/internal/publicrelease	0.007s
ok  	github.com/carlosframework/platform/internal/registry	3.669s
ok  	github.com/carlosframework/platform/internal/relay	7.215s
ok  	github.com/carlosframework/platform/internal/release	18.613s
ok  	github.com/carlosframework/platform/internal/rollout	1.501s
ok  	github.com/carlosframework/platform/internal/rolloutwords	0.003s
ok  	github.com/carlosframework/platform/internal/rootaws	0.007s
ok  	github.com/carlosframework/platform/internal/s3compat	0.004s
ok  	github.com/carlosframework/platform/internal/safefile	0.004s
ok  	github.com/carlosframework/platform/internal/schedule	2.224s
ok  	github.com/carlosframework/platform/internal/sim/hostlife	2.403s
ok  	github.com/carlosframework/platform/internal/sim/litestreamfacts	4.189s
ok  	github.com/carlosframework/platform/internal/status	0.041s
ok  	github.com/carlosframework/platform/internal/steering	0.005s
ok  	github.com/carlosframework/platform/internal/stopevidence	0.084s
ok  	github.com/carlosframework/platform/internal/svcrole	0.002s
ok  	github.com/carlosframework/platform/internal/telemetry	0.004s
ok  	github.com/carlosframework/platform/internal/termfence	0.004s
ok  	github.com/carlosframework/platform/internal/tunnelclient	0.004s
ok  	github.com/carlosframework/platform/internal/unixdial	0.004s
ok  	github.com/carlosframework/platform/internal/updatewords	0.002s
ok  	github.com/carlosframework/platform/internal/usage	0.004s
ok  	github.com/carlosframework/platform/internal/version	0.001s
ok  	github.com/carlosframework/platform/internal/vet	1.548s
=== cross ===
--- shard 0 of 5 ---
cross-compiling darwin/arm64
--- shard 1 of 5 ---
cross-compiling darwin/amd64
--- shard 2 of 5 ---
cross-compiling linux/arm64
--- shard 3 of 5 ---
cross-compiling windows/amd64
--- shard 4 of 5 ---
cross-compiling windows/arm64
=== minio-contract ===
minio contract: arm64, minio RELEASE.2025-09-07T16-13-09Z, mc RELEASE.2025-08-13T08-35-41Z
  minio: cached, digest verified
  mc: cached, digest verified
  minio healthy on 127.0.0.1:20999
Added `local` successfully.
Bucket created successfully `local/contract-bucket`.
Bucket created successfully `local/denied-bucket`.
Created policy `contract-only` successfully.
Added user `scopeduser` successfully.
Attached Policies: [contract-only]
To User: scopeduser
  buckets and scoped user ready
=== RUN   TestDeliveredContractMinio
=== RUN   TestDeliveredContractMinio/put
=== RUN   TestDeliveredContractMinio/get_round-trips_the_body
=== RUN   TestDeliveredContractMinio/list_shows_the_key
=== RUN   TestDeliveredContractMinio/head_and_list_agree_on_the_version
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_its_own_object
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict
=== RUN   TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed
=== RUN   TestDeliveredContractMinio/delete_removes_it
=== RUN   TestDeliveredContractMinio/cross-bucket_denial
--- PASS: TestDeliveredContractMinio (0.02s)
    --- PASS: TestDeliveredContractMinio/put (0.00s)
    --- PASS: TestDeliveredContractMinio/get_round-trips_the_body (0.00s)
    --- PASS: TestDeliveredContractMinio/list_shows_the_key (0.00s)
    --- PASS: TestDeliveredContractMinio/head_and_list_agree_on_the_version (0.00s)
    --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object (0.00s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging (0.00s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload (0.00s)
    --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route (0.01s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload (0.00s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive (0.00s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed (0.00s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed (0.00s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict (0.00s)
        --- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed (0.00s)
    --- PASS: TestDeliveredContractMinio/delete_removes_it (0.00s)
    --- PASS: TestDeliveredContractMinio/cross-bucket_denial (0.00s)
PASS
ok  	github.com/carlosframework/platform/internal/objstore	0.023s
=== RUN   TestT4StorageProbeMinio
2026/10/12 09:30:23 moves: T4: STORE: list: operation error S3: ListObjectsV2, https response error StatusCode: 403, RequestID: 18DDBE437138A39B, HostID: 7c420053e0fbcd7f41653fdd92ffa2802c903d05303b1aa15adc2e8bb65d183f, api error SignatureDoesNotMatch: The request signature we calculated does not match the signature you provided. Check your key and signing method.
--- PASS: TestT4StorageProbeMinio (0.01s)
PASS
ok  	github.com/carlosframework/platform/internal/host	0.016s
minio contract: PASS
diff --git a/internal/host/movesfloor_linux.go b/internal/host/movesfloor_linux.go
index 88deb6e81..29a88713d 100644
--- a/internal/host/movesfloor_linux.go
+++ b/internal/host/movesfloor_linux.go
@@ -13,6 +13,7 @@ import (
"time"
"github.com/carlosframework/platform/internal/safefile"
+ "golang.org/x/sys/unix"
)
// movesDir is <DataDir>/.moves, opened with O_NOFOLLOW below a data dir
@@ -20,9 +21,10 @@ import (
// symlink the sandboxed agent planted. Every operation on the floor files
// goes through one: the lock and the writes are all relative to its fd.
type movesDir struct {
- fd int
- path string
- lock *os.File
+ fd int
+ path string
+ lock *os.File
+ lockName string
}
// floorLockFile serialises every writer of the floor files (root's apply,
@@ -95,32 +97,168 @@ func (d *movesDir) Lock() error { return d.lockNamed(floorLockFile, floorLockWai
// read-only fd can be flocked), so the agent can lock a file root created
// and root a file the agent created; O_NOFOLLOW and O_NONBLOCK so a symlink
// or a FIFO planted in its place cannot redirect or park root. The fd is
-// kept until Close, which releases the lock. One lock per movesDir.
+// kept until Close (or unlockRemoving), which releases the lock. One lock
+// per movesDir.
+//
+// A holder may remove the file it holds before releasing it
+// (unlockRemoving, the held-stage lock). A waiter that opened that file
+// would then lock a file no longer at the name, beside whoever has locked
+// the new one there. So once it has the flock it checks that the name still
+// names the file it locked (fstat of the fd against an fstatat of the name)
+// and, when the name is gone or names another file, closes it and starts
+// again on whatever is there now. The same removal can land inside the
+// open itself (the name vanishes, or the file opened has no link left);
+// that is retried too, while a symlink, a FIFO or a second link at the name
+// is still refused. Each retry pauses (lockNamedRetryPause, capped at what
+// is left of the wait), and none starts once the wait has passed: the call
+// fails with busy then, an open race included, and holds nothing.
+//
+// The deadline bounds the wait; correctness does not depend on it. A flock
+// taken just after the deadline (the check and the syscall are not one
+// step) is still a valid, exclusive lock, and is kept.
func (d *movesDir) lockNamed(name string, wait time.Duration, busy error) error {
- f, err := safefile.OpenFileAt(d.fd, name, os.O_RDONLY|os.O_CREATE, 0o644)
- if err != nil {
- return fmt.Errorf("open %s: %w", filepath.Join(d.path, name), err)
- }
- fd := int(f.Fd())
deadline := time.Now().Add(wait)
+ timedOut := func() error { return fmt.Errorf("lock %s: %w", filepath.Join(d.path, name), busy) }
for {
- err := syscall.Flock(fd, syscall.LOCK_EX|syscall.LOCK_NB)
- if err == nil {
- d.lock = f
- return nil
+ if time.Now().After(deadline) {
+ return timedOut()
}
- if !errors.Is(err, syscall.EWOULDBLOCK) {
+ f, err := lockNamedOpenAt(d.fd, name, os.O_RDONLY|os.O_CREATE, 0o644)
+ if err != nil {
+ if !d.removedDuringOpen(name, err) {
+ return fmt.Errorf("open %s: %w", filepath.Join(d.path, name), err)
+ }
+ // A holder's removal landed inside the open: start again.
+ lockNamedPause(deadline)
+ continue
+ }
+ lockNamedOpened(name)
+ if err := flockWithin(f, deadline); err != nil {
f.Close()
+ if errors.Is(err, syscall.EWOULDBLOCK) {
+ return timedOut()
+ }
return fmt.Errorf("lock %s: %w", filepath.Join(d.path, name), err)
}
- if time.Now().After(deadline) {
+ same, err := d.names(name, f)
+ if err != nil {
f.Close()
- return fmt.Errorf("lock %s: %w", filepath.Join(d.path, name), busy)
+ return fmt.Errorf("lock %s: %w", filepath.Join(d.path, name), err)
+ }
+ if same {
+ d.lock, d.lockName = f, name
+ return nil
+ }
+ // The holder before us removed this file as it released it. Start
+ // again, within the wait: the publication waits inside the
+ // registry's freeze, so a name that keeps changing ends in busy.
+ f.Close()
+ lockNamedPause(deadline)
+ }
+}
+
+// lockNamedPause sleeps lockNamedRetryPause, or what is left of the wait
+// when that is less.
+func lockNamedPause(deadline time.Time) {
+ if left := time.Until(deadline); left > 0 {
+ time.Sleep(min(lockNamedRetryPause, left))
+ }
+}
+
+// flockWithin takes an exclusive flock on f, polling until deadline, and
+// once the deadline has passed stops trying and returns EWOULDBLOCK. A
+// flock that lands just after the deadline is kept: it is a valid,
+// exclusive lock, and the deadline only bounds the wait.
+func flockWithin(f *os.File, deadline time.Time) error {
+ for {
+ if time.Now().After(deadline) {
+ return syscall.EWOULDBLOCK
+ }
+ err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)
+ if err == nil || !errors.Is(err, syscall.EWOULDBLOCK) {
+ return err
}
time.Sleep(floorLockPoll)
}
}
+// names reports whether name in the directory is still the file f has
+// open: a regular file at the name with f's device and inode. A name that
+// is gone is false, not an error.
+func (d *movesDir) names(name string, f *os.File) (bool, error) {
+ var held, now unix.Stat_t
+ if err := unix.Fstat(int(f.Fd()), &held); err != nil {
+ return false, err
+ }
+ err := unix.Fstatat(d.fd, name, &now, unix.AT_SYMLINK_NOFOLLOW)
+ if errors.Is(err, unix.ENOENT) {
+ return false, nil
+ }
+ if err != nil {
+ return false, err
+ }
+ return now.Dev == held.Dev && now.Ino == held.Ino, nil
+}
+
+// removedDuringOpen reports whether err, from opening name, is the race
+// with a holder removing it: not an errno other than ENOENT (a real refusal,
+// such as EACCES or ELOOP), and the name is now gone or a regular file with
+// one link (a fresh lock file). Anything else at the name keeps the open's
+// refusal, and so does a .moves that has itself been removed (no create
+// can succeed in it, so waiting out the lock's wait would only delay the
+// error).
+func (d *movesDir) removedDuringOpen(name string, err error) bool {
+ var errno syscall.Errno
+ if errors.As(err, &errno) && errno != syscall.ENOENT {
+ return false
+ }
+ var st unix.Stat_t
+ if derr := unix.Fstat(d.fd, &st); derr != nil || st.Nlink == 0 {
+ return false
+ }
+ serr := unix.Fstatat(d.fd, name, &st, unix.AT_SYMLINK_NOFOLLOW)
+ if errors.Is(serr, unix.ENOENT) {
+ return true
+ }
+ return serr == nil && st.Mode&unix.S_IFMT == unix.S_IFREG && st.Nlink == 1
+}
+
+// unlockRemoving removes the lock file while still holding its flock, then
+// releases the lock (closing its fd). The name is removed only while it
+// still names the file held, so another holder's file is never taken. A
+// waiter on the removed file finds the name gone after its flock and starts
+// again (lockNamed), so two holders never exist at once. The lock is
+// released whatever the removal does; an error is the removal's.
+func (d *movesDir) unlockRemoving() error {
+ if d.lock == nil {
+ return nil
+ }
+ f, name := d.lock, d.lockName
+ d.lock, d.lockName = nil, ""
+ defer f.Close()
+ same, err := d.names(name, f)
+ if err != nil || !same {
+ return err
+ }
+ if err := unix.Unlinkat(d.fd, name, 0); err != nil && !errors.Is(err, unix.ENOENT) {
+ return fmt.Errorf("remove %s: %w", filepath.Join(d.path, name), err)
+ }
+ return nil
+}
+
+// lockNamedRetryPause is the pause between lockNamed's retries after a
+// holder's removal, capped at what is left of the wait.
+const lockNamedRetryPause = 10 * time.Millisecond
+
+// lockNamedOpenAt opens the lock file for lockNamed. A test seam: a test
+// can make every open meet the race with a holder's removal.
+var lockNamedOpenAt = safefile.OpenFileAt
+
+// lockNamedOpened runs in lockNamed after each open of the lock file and
+// before its flock. A test seam: the window in which the file a waiter has
+// open can be removed by the holder before it and replaced by a new one.
+var lockNamedOpened = func(name string) {}
+
// WriteDurable writes data to .moves/name the way writeDurable does (a
// temp file, fsync, rename, fsync of the directory), all relative to the
// directory's fd. The temp name is random, so two writers never share one.
diff --git a/internal/host/movesfloor_other.go b/internal/host/movesfloor_other.go
index c21c86f92..dbb5ff9c0 100644
--- a/internal/host/movesfloor_other.go
+++ b/internal/host/movesfloor_other.go
@@ -22,6 +22,8 @@ func (d *movesDir) Lock() error { return errSystemUnsupported }
func (d *movesDir) lockNamed(string, time.Duration, error) error { return errSystemUnsupported }
+func (d *movesDir) unlockRemoving() error { return errSystemUnsupported }
+
func (d *movesDir) WriteDurable(name string, data []byte, mode os.FileMode) error {
return errSystemUnsupported
}
diff --git a/internal/host/movesheldlock.go b/internal/host/movesheldlock.go
index 4e12216dd..b60954a86 100644
--- a/internal/host/movesheldlock.go
+++ b/internal/host/movesheldlock.go
@@ -3,6 +3,7 @@ package host
import (
"errors"
"fmt"
+ "log"
"os"
"time"
@@ -28,6 +29,17 @@ import (
// call (D3's admit, which may read the bucket, runs before it on both
// sides), so it never inverts the order and never holds the registry's
// freeze for long.
+//
+// The file is removed when the stage finishes (region HA M-K25, #236), by
+// the holder, while it still holds the flock (unlockRemoving). Main's
+// empty-installation census refuses any file naming the host being
+// installed, so a lock left behind would refuse every later from-empty
+// install of that host on this box. A waiter that opened the removed file
+// re-checks the name after its flock and starts again on the new one
+// (lockNamed), so two holders never exist at once. A holder killed
+// mid-stage never removes it: the file stays, the census refuses that host
+// (region HA's CensusTree(".moves") fallback refuses only that host), and
+// the next stage of the host locks the same file and then removes it.
// heldLockWait bounds the wait for a held-stage lock. The publication waits
// for it inside the registry's freeze, after a claim read bounded by
@@ -45,9 +57,12 @@ var errHeldLockBusy = errors.New("another process holds this host's held-stage l
func heldStageLockFile(host string) string { return "held-" + host + ".lock" }
// withHeldStageLock runs fn holding host's held-stage lock, making .moves
-// when it is missing. An error opening or locking is returned without
-// running fn. Off Linux it always refuses (openMovesDir does), which fails
-// T7 and the publication closed; production boxes are Linux.
+// when it is missing, and removes the lock file before releasing it,
+// whatever fn returns. An error opening or locking is returned without
+// running fn. A removal that fails is logged and leaves the file as a crash
+// would; fn's result stands. Off Linux it always refuses (openMovesDir
+// does), which fails T7 and the publication closed; production boxes are
+// Linux.
func withHeldStageLock(dataDir, host string, fn func() error) error {
if !controlname.ValidHostShape(host) {
return fmt.Errorf("held-stage lock: malformed host %q", host)
@@ -63,5 +78,9 @@ func withHeldStageLock(dataDir, host string, fn func() error) error {
if err := d.lockNamed(heldStageLockFile(host), heldLockWait, errHeldLockBusy); err != nil {
return err
}
- return fn()
+ err = fn()
+ if rerr := d.unlockRemoving(); rerr != nil {
+ log.Printf("moves: %s: held-stage lock: %v", host, rerr)
+ }
+ return err
}
diff --git a/internal/host/movesheldlock_cleanup_test.go b/internal/host/movesheldlock_cleanup_test.go
new file mode 100644
index 000000000..1ab34302d
--- /dev/null
+++ b/internal/host/movesheldlock_cleanup_test.go
@@ -0,0 +1,391 @@
+//go:build linux
+
+package host
+
+import (
+ "errors"
+ "os"
+ "path/filepath"
+ "sync/atomic"
+ "syscall"
+ "testing"
+ "time"
+
+ "github.com/carlosframework/platform/internal/hostbox"
+ "github.com/carlosframework/platform/internal/hostclaim"
+ "github.com/carlosframework/platform/internal/registry"
+ "github.com/carlosframework/platform/internal/release"
+)
+
+// The held-stage lock's file is removed when the stage finishes (region HA
+// M-K25, answered by moves on #236): main's empty-installation census
+// refuses a file that names the host being installed, so a lock left
+// behind would refuse every later from-empty install of that host on the
+// box that held a stage for it.
+
+// heldLockCensus runs main's empty-installation census for a from-empty
+// install of host on a box whose data root is data, with the production
+// classification (claimGlobalSharedPaths) and any extra data entries.
+func heldLockCensus(t *testing.T, data, host string, extra ...string) error {
+ t.Helper()
+ base := t.TempDir()
+ inventory, err := hostbox.OpenStopInventoryDB(filepath.Join(base, "inventory.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer inventory.Close()
+ reg, err := registry.Open(filepath.Join(base, "registry.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer reg.Close()
+ launches, err := hostbox.OpenLaunchJournal(filepath.Join(base, "launches.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer launches.Close()
+ r := ClaimRuntimeResources{DataDir: data, OptDir: filepath.Join(base, "opt"), UnitsDir: filepath.Join(base, "units")}
+ for _, dir := range []string{r.OptDir, r.UnitsDir} {
+ if err := os.Mkdir(dir, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ }
+ shared := claimGlobalSharedPaths(r)
+ shared["data"] = append(shared["data"], extra...)
+ c := hostbox.EmptyInstallationCensus{Inventory: inventory, Registry: reg, Launches: launches, Box: "box", Enrollment: "enrollment", DirectoryRevision: "1",
+ Roots: map[string]string{"data": r.DataDir, "opt": r.OptDir, "units": r.UnitsDir}, SharedPaths: shared}
+ obs := hostclaim.Observed{Host: host, Claim: &hostclaim.Claim{Holder: &hostclaim.Holder{Incarnation: "inc", Stamp: 1}}}
+ return c.CheckLocal(t.Context(), obs, hostbox.RecordSelection{Host: host, Incarnation: "inc"})
+}
+
+func heldLockInode(t *testing.T, path string) uint64 {
+ t.Helper()
+ info, err := os.Lstat(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return info.Sys().(*syscall.Stat_t).Ino
+}
+
+func TestHeldStageLockIsRemovedWhenTheStageFinishes(t *testing.T) {
+ data := t.TempDir()
+ path := filepath.Join(data, ".moves", heldStageLockFile(hsHost))
+ if err := heldLockCensus(t, data, hsHost); err != nil {
+ t.Fatalf("census before any stage: %v", err)
+ }
+ for _, result := range []error{nil, errors.New("the stage failed")} {
+ err := withHeldStageLock(data, hsHost, func() error {
+ if _, err := os.Lstat(path); err != nil {
+ t.Fatalf("no lock file while the lock is held: %v", err)
+ }
+ return result
+ })
+ if err != result {
+ t.Fatalf("stage: %v, want %v", err, result)
+ }
+ if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("after a stage that returned %v, %s remains: %v", result, path, err)
+ }
+ }
+ // The publication path: heldStageAuthority's then, inside the freeze.
+ s := release.NewDirStore(t.TempDir())
+ hsClaim(t, s, movingOp())
+ hsReport(t, s, hsOp, "v7")
+ auth := heldStageAuthority(testRegistry(t), s, hsBox, data)
+ published := false
+ if err := auth(hsRequest(), func() error { published = true; return nil }); err != nil || !published {
+ t.Fatalf("publication: %v (ran %v)", err, published)
+ }
+ if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("after a publication, %s remains: %v", path, err)
+ }
+ // A from-empty install of that host on this box passes main's census.
+ if err := heldLockCensus(t, data, hsHost); err != nil {
+ t.Fatalf("census after held stages of the host: %v", err)
+ }
+}
+
+// A waiter that opened the file a holder then removes must not lock that
+// file and run beside whoever holds the new one at the same name: it
+// re-checks the name after its flock and retries.
+func TestHeldStageLockWaiterOnARemovedFileRetries(t *testing.T) {
+ prevWait, prevHook := heldLockWait, lockNamedOpened
+ t.Cleanup(func() { heldLockWait, lockNamedOpened = prevWait, prevHook })
+ heldLockWait = 10 * time.Second
+ data := t.TempDir()
+ path := filepath.Join(data, ".moves", heldStageLockFile(hsHost))
+ var holders, overlaps atomic.Int32
+ hold := func(entered chan<- struct{}, leave <-chan struct{}) func() error {
+ return func() error {
+ if holders.Add(1) != 1 {
+ overlaps.Add(1)
+ }
+ defer holders.Add(-1)
+ close(entered)
+ if leave != nil {
+ <-leave
+ }
+ return nil
+ }
+ }
+
+ // The first holder holds the lock on the first file.
+ in1, out1, done1 := make(chan struct{}), make(chan struct{}), make(chan error, 1)
+ go func() { done1 <- withHeldStageLock(data, hsHost, hold(in1, out1)) }()
+ <-in1
+
+ // The waiter opens that file and is parked before its flock.
+ opened, resume := make(chan struct{}), make(chan struct{})
+ var opens atomic.Int32
+ lockNamedOpened = func(name string) {
+ if name == heldStageLockFile(hsHost) && opens.Add(1) == 1 {
+ close(opened)
+ <-resume
+ }
+ }
+ inW, doneW := make(chan struct{}), make(chan error, 1)
+ go func() { doneW <- withHeldStageLock(data, hsHost, hold(inW, nil)) }()
+ <-opened
+
+ // The first holder finishes: it removes its file and releases.
+ close(out1)
+ if err := <-done1; err != nil {
+ t.Fatal(err)
+ }
+ if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) {
+ close(resume)
+ t.Fatalf("the first holder left %s: %v", path, err)
+ }
+
+ // A second holder arrives and locks a new file at the same name.
+ in2, out2, done2 := make(chan struct{}), make(chan struct{}), make(chan error, 1)
+ go func() { done2 <- withHeldStageLock(data, hsHost, hold(in2, out2)) }()
+ <-in2
+
+ // The waiter resumes. Its fd names the removed file, which nobody
+ // holds: it must not run until the second holder is done.
+ close(resume)
+ select {
+ case <-inW:
+ close(out2)
+ t.Fatalf("the waiter ran while the second holder held the lock (overlaps %d)", overlaps.Load())
+ case <-time.After(500 * time.Millisecond):
+ }
+ close(out2)
+ if err := <-done2; err != nil {
+ t.Fatal(err)
+ }
+ select {
+ case <-inW:
+ case <-time.After(5 * time.Second):
+ t.Fatal("the waiter never took the lock after the second holder released it")
+ }
+ if err := <-doneW; err != nil {
+ t.Fatal(err)
+ }
+ if n := overlaps.Load(); n != 0 {
+ t.Fatalf("%d holders ran beside another", n)
+ }
+ if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("after every holder finished, %s remains: %v", path, err)
+ }
+}
+
+// A holder killed mid-stage never reaches the removal, so its file stays
+// (the existing behaviour). Main's census then refuses an install of that
+// host; region HA's fallback (CensusTree(".moves")) refuses that host only.
+// The next stage of the host locks the file the crash left and removes it.
+func TestHeldStageLockLeftByACrashIsReusedThenRemoved(t *testing.T) {
+ data := t.TempDir()
+ path := filepath.Join(data, ".moves", heldStageLockFile(hsHost))
+ if err := os.Mkdir(filepath.Dir(path), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(path, nil, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ left := heldLockInode(t, path)
+ if err := heldLockCensus(t, data, hsHost); !errors.Is(err, hostbox.ErrTypedInventory) {
+ t.Fatalf("main's census with a crashed stage's lock: %v, want ErrTypedInventory", err)
+ }
+ if err := heldLockCensus(t, data, hsHost, hostbox.CensusTree(".moves")); !errors.Is(err, hostbox.ErrTypedInventory) {
+ t.Fatalf("the fallback census for the lock's host: %v, want ErrTypedInventory", err)
+ }
+ if err := heldLockCensus(t, data, "other.example.com", hostbox.CensusTree(".moves")); err != nil {
+ t.Fatalf("the fallback census for another host: %v", err)
+ }
+ ran := false
+ if err := withHeldStageLock(data, hsHost, func() error {
+ ran = true
+ if got := heldLockInode(t, path); got != left {
+ t.Fatalf("the next stage locked a new file (inode %d), not the one the crash left (%d)", got, left)
+ }
+ return nil
+ }); err != nil || !ran {
+ t.Fatalf("the next stage: %v (ran %v)", err, ran)
+ }
+ if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("the next stage left %s: %v", path, err)
+ }
+ if err := heldLockCensus(t, data, hsHost); err != nil {
+ t.Fatalf("census after the next stage: %v", err)
+ }
+}
+
+// A holder's removal can land inside a waiter's open: the name vanishes
+// between the create and the reopen, or the file reopened has no link
+// left. lockNamed opens again then, but keeps the refusal of anything
+// else at the name.
+func TestHeldStageLockOpenRetriesOnlyTheRemovalRace(t *testing.T) {
+ data := t.TempDir()
+ d, err := openMovesDir(data, true)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer d.Close()
+ at := func(name string) string { return filepath.Join(data, ".moves", name) }
+ if err := os.WriteFile(at("fresh.lock"), nil, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(at("linked.lock"), nil, 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Link(at("linked.lock"), at("second.lock")); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(t.TempDir(), at("planted.lock")); err != nil {
+ t.Fatal(err)
+ }
+ if err := syscall.Mkfifo(at("fifo.lock"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ unlinked := &os.PathError{Op: "open", Path: "x", Err: errors.New("not a regular file with one link")}
+ for _, c := range []struct {
+ name string
+ err error
+ want bool
+ }{
+ {"gone.lock", &os.PathError{Op: "open", Path: "x", Err: syscall.ENOENT}, true},
+ {"gone.lock", unlinked, true},
+ {"fresh.lock", unlinked, true},
+ {"fresh.lock", &os.PathError{Op: "open", Path: "x", Err: syscall.EACCES}, false},
+ {"linked.lock", unlinked, false},
+ {"planted.lock", unlinked, false},
+ {"fifo.lock", unlinked, false},
+ } {
+ if got := d.removedDuringOpen(c.name, c.err); got != c.want {
+ t.Errorf("%s after %v: retry %v, want %v", c.name, c.err, got, c.want)
+ }
+ }
+ // A .moves removed under an open movesDir: nothing can be created in
+ // it, so the open's error stands at once.
+ gone := t.TempDir()
+ g, err := openMovesDir(gone, true)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer g.Close()
+ if err := os.Remove(filepath.Join(gone, ".moves")); err != nil {
+ t.Fatal(err)
+ }
+ if g.removedDuringOpen("gone.lock", &os.PathError{Op: "open", Path: "x", Err: syscall.ENOENT}) {
+ t.Error("a removed .moves was retried")
+ }
+}
+
+// Fix round 1: the re-check's retries are bounded by the one wait. With
+// every file the waiter opens replaced at the name before its flock, each
+// re-check finds another file there; the call still ends with the busy
+// error once the wait has passed, and never runs fn. Publication waits
+// inside the registry's freeze, so the wait must hold.
+func TestHeldStageLockRetriesEndWithTheWait(t *testing.T) {
+ prevWait, prevHook := heldLockWait, lockNamedOpened
+ const wait = 200 * time.Millisecond
+ heldLockWait = wait
+ data := t.TempDir()
+ path := filepath.Join(data, ".moves", heldStageLockFile(hsHost))
+ var stop atomic.Bool
+ var replaced atomic.Int32
+ lockNamedOpened = func(name string) {
+ if name != heldStageLockFile(hsHost) || stop.Load() {
+ return
+ }
+ if err := os.Remove(path); err != nil {
+ return
+ }
+ if err := os.WriteFile(path, nil, 0o644); err == nil {
+ replaced.Add(1)
+ }
+ }
+ var ran atomic.Bool
+ done := make(chan error, 1)
+ start := time.Now()
+ go func() { done <- withHeldStageLock(data, hsHost, func() error { ran.Store(true); return nil }) }()
+ t.Cleanup(func() {
+ // A waiter still retrying is let through before the seam goes.
+ stop.Store(true)
+ select {
+ case <-done:
+ case <-time.After(10 * time.Second):
+ }
+ heldLockWait, lockNamedOpened = prevWait, prevHook
+ })
+ var err error
+ select {
+ case err = <-done:
+ done <- err
+ case <-time.After(5 * time.Second):
+ t.Fatalf("still retrying %v after a %v wait (%d files replaced, ran %v)", time.Since(start), wait, replaced.Load(), ran.Load())
+ }
+ elapsed := time.Since(start)
+ if !errors.Is(err, errHeldLockBusy) {
+ t.Fatalf("with every opened file replaced: %v, want errHeldLockBusy", err)
+ }
+ if ran.Load() {
+ t.Fatal("fn ran without the lock")
+ }
+ if elapsed > wait+2*time.Second {
+ t.Fatalf("gave up after %v, want about %v", elapsed, wait)
+ }
+ if replaced.Load() < 2 {
+ t.Fatalf("only %d files were replaced; the re-check was not exercised", replaced.Load())
+ }
+ // Fix round 2: replacement retries pause (lockNamedRetryPause, capped
+ // at what is left of the wait), so they cannot spin. A pause is a floor
+ // on each retry's time, so this bound holds however loaded the box.
+ if most := int32(wait/lockNamedRetryPause) + 2; replaced.Load() > most {
+ t.Fatalf("%d replacement retries in a %v wait, want at most %d", replaced.Load(), wait, most)
+ }
+}
+
+// Fix round 2: an open that meets a holder's removal after the wait has
+// passed ends in the caller's busy error, like every other way the wait
+// runs out, never the open's own error. Every open here meets that race:
+// it fails as an unlinked file does and the name is gone.
+func TestHeldStageLockOpenRaceAfterTheWaitIsBusy(t *testing.T) {
+ prevWait, prevOpen := heldLockWait, lockNamedOpenAt
+ t.Cleanup(func() { heldLockWait, lockNamedOpenAt = prevWait, prevOpen })
+ const wait = 150 * time.Millisecond
+ heldLockWait = wait
+ var opens atomic.Int32
+ lockNamedOpenAt = func(dir int, name string, flags int, mode os.FileMode) (*os.File, error) {
+ opens.Add(1)
+ return nil, &os.PathError{Op: "open", Path: name, Err: errors.New("not a regular file with one link")}
+ }
+ ran := false
+ start := time.Now()
+ err := withHeldStageLock(t.TempDir(), hsHost, func() error { ran = true; return nil })
+ elapsed := time.Since(start)
+ if !errors.Is(err, errHeldLockBusy) {
+ t.Fatalf("every open raced a removal until past the wait: %v, want errHeldLockBusy", err)
+ }
+ if ran {
+ t.Fatal("fn ran without the lock")
+ }
+ if elapsed < wait || elapsed > wait+time.Second {
+ t.Fatalf("gave up after %v, want about %v", elapsed, wait)
+ }
+ if opens.Load() < 2 {
+ t.Fatalf("only %d opens; the race was not retried", opens.Load())
+ }
+}