Skip to content

Commit a9f4902

Browse files
authored
Merge pull request blooop#577 from blooop/fix/575-reclaim-the-launch-locks
fix: a per-workspace launch lock was never reclaimed by anything but --purge
2 parents b5d0300 + 1507148 commit a9f4902

14 files changed

Lines changed: 1392 additions & 87 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,61 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6969
now say what it does -- recreate the container and its volumes -- and
7070
`docs/cli.md` and `docs/workspaces.md` gained the sentence that `rm` is the only
7171
verb which refreshes git state.
72+
73+
### Fixed
74+
75+
- **`dl --prune` reclaims the per-workspace launch locks**, which nothing short
76+
of `--purge` removing the whole cache directory had ever reached. Two `dl` runs
77+
launching one workspace serialize on an empty file under
78+
`<cache>/devlaunch/launch-locks/<workspace-id>.lock`; every workspace ever
79+
launched left one and no removal path took it away. Measured on one host: 18 of
80+
26 entries named no workspace `devpod list` still returns, against 8 live
81+
workspaces, the oldest three weeks old.
82+
83+
Zero bytes of disk, and still the thing worth fixing. A directory that only
84+
ever grows is one nobody can read as a description of anything, and "nothing
85+
devlaunch causes to exist accumulates unreclaimed" is the rule the rest of the
86+
cleanup contract is written to.
87+
88+
The precondition is the volumes' precondition, asked of the same listing: no
89+
workspace `devpod list` returns carries that id, re-asked under the acting
90+
pass's own second listing so the approved set can shrink between the plan and
91+
the act and can never grow.
92+
93+
- **Unlinking a lock file is safe now, which it was not before.** Removing an
94+
`flock`'d file is the self-defeating move: a process holding the old inode
95+
excludes nobody, while new arrivals lock a fresh file at the same path and walk
96+
past it, and two `devpod up`s of one workspace run against each other. The rule
97+
used to be that no lock file is ever deleted, which is what made the leak
98+
above permanent.
99+
100+
What replaced the rule is one question asked after every `flock`, on both
101+
sides: is the path still naming the inode I just locked? An acquisition that
102+
has lost its file queues again against the live one rather than believing
103+
itself alone, and a sweep that has lost its file re-opens rather than unlinking
104+
whatever the path names by then. Both halves are needed. A sweep that only took
105+
the lock would still have unlinked a live launch's file, because an inode
106+
somebody else already unlinked has no holders and locking one always succeeds.
107+
108+
The unlink is then performed while holding the lock, and never queues for it: a
109+
lock a launch is holding is left standing, reported as held, and reclaimed by
110+
the next prune. That is not counted as a failure and does not change the exit
111+
code, because it is the guard working.
112+
113+
**`--purge` is not the only other unlinker, it is the other unlinker that holds
114+
no lock.** It removes the cache directory entire, launch locks with it, which is
115+
what makes "somebody replaced this path" an ordinary event here rather than a
116+
devlaunch bug, and why the question above is asked every time rather than as a
117+
belt over a brace.
118+
119+
- **devpod's own per-workspace lock is left alone, deliberately**, and the
120+
cleanup contract now says so in a table row rather than by omission. The same
121+
measurement found 70 of 78 stale entries under
122+
`~/.devpod/contexts/default/locks`. They stay: it is another program's lock,
123+
taken by processes devlaunch does not run, and the revalidation above is a
124+
promise devlaunch can only make about a file only devlaunch opens. It is the
125+
same file `dl <ws> kill` has always been careful never to unlink.
126+
72127
## [0.31.0] - 2026-09-04
73128

74129
### Added

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ instead. [docs/cli.md](docs/cli.md) has the full `--rm` contract, including whic
253253
| `dl --ls` | List every workspace |
254254
| `dl --ls --json` | The same, machine-readable, with what each workspace would lose if deleted |
255255
| `dl --ls --size` | Add what deleting each one would free. Opt-in: it walks every file |
256-
| `dl --prune` | Remove the clone directories no workspace opens any more, the agent git worktrees inside the clones it keeps, and the volumes of workspaces devpod has forgotten |
256+
| `dl --prune` | Remove the clone directories no workspace opens any more, the agent git worktrees inside the clones it keeps, and the volumes and launch locks of workspaces devpod has forgotten |
257257
| `dl --reconcile` | Re-point workspaces whose recorded source folder went missing. Deletes nothing |
258258
| `dl --purge` | Remove devlaunch's own workspaces and caches |
259259
| `dl --install` | Install shell completions, and the `dl-herdr-shell` name a herdr pane opens through |
@@ -405,7 +405,7 @@ different jobs:
405405

406406
| Command | Takes | Leaves |
407407
|---|---|---|
408-
| `dl --prune` | Clone directories no workspace opens, collectable agent git worktrees inside the ones it keeps, and the volumes of workspaces devpod no longer lists | Every workspace, container and image |
408+
| `dl --prune` | Clone directories no workspace opens, collectable agent git worktrees inside the ones it keeps, and the volumes and launch locks of workspaces devpod no longer lists | Every workspace, container and image |
409409
| `dl --purge` | The workspaces devlaunch created, and its caches | Workspaces it did not create, each named with its source before it asks |
410410
| `dl --reconcile` | Nothing | Repairs records that stopped matching the disk |
411411

‎docs/cleanup.md‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,9 @@ until now nothing removed them: measured on one host, **52 clone directories for
304304
all-or-nothing: the only way to get the 4 GB back was to destroy the 7.86 GB
305305
too, and every bare cache with it.
306306

307-
`dl --prune` removes exactly the clone directories no live workspace opens, and
308-
reclaims the Docker volumes of workspaces devpod no longer lists. It never
307+
`dl --prune` removes exactly the clone directories no live workspace opens,
308+
reclaims the Docker volumes of workspaces devpod no longer lists, and reclaims
309+
the launch locks of workspaces devpod no longer lists. It never
309310
deletes a devpod workspace, a container or an image, never
310311
touches a repo's `.bare` cache (0.08 GB for seven repos, and it is what makes
311312
the next clone of a repo fast), and never looks outside
@@ -637,6 +638,57 @@ walk of a 12000-file environment, and the listing is a read-only command people
637638
run casually; a derivative is never a reason a site stands, so the listing loses
638639
no answer by not asking.
639640

641+
#### The per-workspace launch locks
642+
643+
Two `dl` runs launching one workspace serialize on a lock file under
644+
`<cache>/devlaunch/launch-locks/<workspace-id>.lock`. It is empty: the only thing
645+
it carries is the `flock` the kernel holds on it while a launch is inside its
646+
critical section, and the kernel drops that when the process ends, however it
647+
ends.
648+
649+
Until [#575](https://github.com/blooop/devlaunch/issues/575) nothing removed one.
650+
Every workspace ever launched left an entry and no removal path took it away, so
651+
the directory only ever grew: measured on one host, **18 of 26 entries named no
652+
workspace `devpod list` still returns**, against 8 live workspaces, the oldest
653+
three weeks old. `dl <ws> rm` did not reach them, `--prune` did not, and
654+
`--purge` only did because it removes the whole cache directory. That is zero
655+
bytes of disk and it is still the thing worth fixing: a directory that only grows
656+
is one nobody can read as a description of anything.
657+
658+
`--prune` reclaims them now, on the same plan and the same `y/N` as everything
659+
above, and the precondition is the volumes' precondition: no workspace `devpod
660+
list` returns carries that id.
661+
662+
| artifact | who reclaims it | what makes that safe |
663+
| --- | --- | --- |
664+
| `<cache>/devlaunch/launch-locks/<id>.lock` | `--prune`, and `--purge` with the rest of the cache | no workspace devpod lists carries that id, re-asked under the acting pass's own listing, **and** nothing holds the lock at the moment of the unlink |
665+
| `~/.devpod/contexts/<ctx>/locks/<id>.workspace.lock` | nobody, deliberately | it is devpod's lock, taken by processes devlaunch does not run. See below |
666+
667+
**Why unlinking a lock file needed a change to how they are taken.** Removing an
668+
`flock`'d file is the classic self-defeating move: a process that opened the old
669+
inode still holds a lock nobody else can see, while new arrivals lock a fresh
670+
file at the same path and walk straight past it, and two `devpod up`s of one
671+
workspace then run against each other. That hazard is the reason the rule used to
672+
be that no lock file is ever deleted. What replaced the rule is a check at the
673+
other end: an acquisition now hands back a guard only for the inode the path
674+
*still names*, so a run whose file went out from under it queues again against
675+
the live one instead of believing itself alone. The unlink itself is performed
676+
while holding the lock, and never blocks for it, so a launch inside its critical
677+
section keeps its file and the sweep says so rather than waiting.
678+
679+
A lock a launch is holding is left standing and is not counted as a failure. It
680+
is the guard working, the run still ends 0, and the next `--prune` finds it free.
681+
682+
**devpod's own lock is not devlaunch's to reclaim.** The same measurement found
683+
70 of 78 stale entries under `~/.devpod/contexts/default/locks`, and they stay.
684+
It is the same file `dl <ws> kill` is careful never to unlink, for the same
685+
reason: the revalidation above is a promise devlaunch makes to itself about a
686+
file only devlaunch opens, and devpod makes no such promise about this one. A
687+
`dl` that removed it would be deciding that another program's mutual exclusion is
688+
devlaunch's to break. The file devpod does leave stale, and which `kill` does
689+
remove, is the busy marker beside it, which is a plain file and not a lock at
690+
all.
691+
640692
#### The disk neither command frees
641693

642694
Both commands end on the same line, in the same words:

0 commit comments

Comments
 (0)