fix: a devpod up no longer outlives the dl that holds its lock - #668
Merged
Merged
Conversation
…s handler The own-group child (devpod up) now gets PR_SET_PDEATHSIG(SIGKILL) in its pre_exec, and checks getppid against the pid read before the fork so a parent that died in between is not missed. aid's --boot-up child gets the same with SIGINT, so the boot's own handler still kills its group and unlinks its token. Linux only; a no-op elsewhere.
…ng kill A delete blocked on devpod's workspace lock now runs the sweep a launch runs (kill::release_the_lock): orphans holding the workspace are signalled, a holder somebody is waiting on is spared, and the line says what the sweep found. Both the launch and the delete sweep again every 12 lock lines (about a minute), since a holder spared once can lose its parent later; a repeat sweep speaks only when it signalled something. Tests: the delete sweeps an orphan (core), the lock-wait count, the launch's repeat sweep, and the rm binary test now expects the sweep's finding.
… its SIGTERM The drain sent SIGTERM to the group and _exited at once. A devpod up that ignored it, or a child of it that did, outlived dl holding the workspace flock. Now the drain waits up to two seconds for the leader (waitpid WNOHANG and poll, both async-signal-safe), then SIGKILLs the group. The wait also gives devpod up its unwind before the kernel's parent-death SIGKILL lands. Test: a Ctrl-C mid-up kills an up child that ignores SIGTERM.
…KILL Removing the drain's wait for the group leader left every interrupt test green. The new test's up writes a marker 0.5 s into its SIGTERM unwind; it fails with the wait removed.
Removing the boot's parent-death SIGINT left every aid test green. aid runs under a shell that leads the pty's session and outlives it, so the kernel's hangup cannot stand in for the parent-death signal. The test fails with the ends_with_this_process call removed.
docs/cli.md still said a delete behind devpod's lock answered the first lock line by naming dl <ws> kill, and said it once. It now prints the launch's notice and sweeps on the first line and every twelfth after, and kill's own delete does the same.
No delete test sent more than one sweep's worth of lock lines, so a delete that swept once and never again passed the suite. This one sends 1 + SWEEP_AGAIN_EVERY lines and counts the process-table reads.
The first sweep spares a holder that a live dl is behind, and a later sweep takes it once that dl is gone. Nothing tested that the later sweep is said, so a gate reduced to `if first` passed in both the launch and the delete. Each now reads a different process table per sweep.
Every delete test fed the sweep an empty table or an orphan, so a sweep that signalled attended holders too passed the delete suite. This one puts a devpod up with a live dl behind it on the lock.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuidePrevents devpod up and aid boot processes from becoming orphaned lock holders, adds automatic orphan sweeping for blocked launches and deletes with periodic retries, and makes interrupt cleanup gracefully terminate before forcing process-group shutdown; the behavior is covered by Linux integration tests and documented for the 0.59.3 release. Sequence diagram for parent-death cleanup of devpod upsequenceDiagram
participant DL as dl or aid
participant Child as devpod up or boot child
participant Kernel as Linux kernel
participant Lock as Workspace lock
DL->>Child: spawn with ends_with_this_process
Child->>Kernel: prctl(PR_SET_PDEATHSIG)
Child->>Kernel: getppid() check
Child->>Lock: acquire workspace lock
DL--xDL: parent exits or is SIGKILLed
Kernel-->>Child: SIGKILL for devpod up
Kernel-->>Child: SIGINT for aid boot
Child->>Lock: release lock and exit
Sequence diagram for periodic orphan lock sweepingsequenceDiagram
participant Launch as devpod up or delete
participant Devpod as devpod
participant LockWait as LockWait
participant Sweep as release_the_lock
participant Holder as Lock holder
Launch->>Devpod: start watched operation
Devpod-->>Launch: Trying to lock workspace
Launch->>LockWait: read(lock line)
LockWait-->>Launch: First
Launch->>Sweep: release_the_lock
Sweep->>Holder: signal orphan holder
Holder-->>Devpod: release workspace lock
loop Every 12 lock lines
Devpod-->>Launch: Trying to lock workspace
Launch->>LockWait: read(lock line)
LockWait-->>Launch: SweepAgain
Launch->>Sweep: release_the_lock
Sweep->>Holder: signal only if now orphaned
end
Sequence diagram for graceful interrupt shutdownsequenceDiagram
actor User
participant DL as dl
participant Group as devpod up process group
participant Kernel as Kernel
User->>DL: SIGINT, SIGTERM, or SIGHUP
DL->>Group: killpg(SIGTERM)
DL->>Group: wait up to 2 seconds
alt group leader exits
Group-->>DL: leader reaped
else timeout
DL->>Group: killpg(SIGKILL)
end
DL->>Kernel: exit
Kernel-->>Group: PDEATHSIG SIGKILL if still running
Flow diagram for blocked delete lock handlingflowchart TD
A[devpod delete or rm waits for workspace lock] --> B[LockWait reads first lock line]
B --> C[release_the_lock sweeps orphan holders]
C --> D{Lock holder has live parent?}
D -->|Yes| E[Preserve holder and continue waiting]
D -->|No| F[Signal orphan holder]
F --> G[Delete continues when lock is released]
E --> H[Repeat sweep every 12 lock lines]
H --> C
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
All four copies of the version move together: Cargo.toml, Cargo.lock, and the README conda badge and dl --version transcript. The bump rides on #668 so the merge that lands the fix is the one that publishes it.
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
An rm, rme or --rm whose sweep spared somebody's live `devpod up` ended its line with "'dl <ws> kill' ends it and deletes the workspace". kill classifies that holder as Standing::ABuild, spares it in the same sweep, and withholds its delete (kill_delete_withheld), so the advice sent the reader to a command that does neither. The delete waiter now says to stop the build in its own terminal and that this rm deletes the workspace once it lets go. The README and docs/cli.md copies this branch added are corrected to match. The launch waiter's sentence is unchanged here.
…n's SIGKILL gone The fake up was `exec sleep 30`, which pdeathsig kills when dl exits, so up_alive: false held without the drain's killpg(SIGKILL). The up now forks a child that records its pid and blocks on it; a forked child does not inherit pdeathsig, and under the TERM row it inherits the ignored SIGTERM, so only the drain's SIGKILL ends it.
… group guarantees
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
On a host, two
devpod upprocesses kept devpod's workspace lock for four hours. Theiraid --boot-upparent had died, and init became their parent. Every laterdl <ws>,dl rmeanddevpod deletethen printedTrying to lock workspaceand waited with no end.devpod updies with itsdl. On Linux, the runner setsPR_SET_PDEATHSIGinpre_execfor a child that leads its own group (ends_with_this_process,devlaunch-runner/src/interrupt.rs). Thedevpod upchild getsSIGKILL. Theaid --boot-upchild getsSIGINT, so its own handler still removes its token file. Afterprctl, the child comparesgetppid()with the pid that the parent read before the fork. If the parent died first, the child exits. On other platforms the function does nothing.rm,rmeand--rmclear an orphan that holds the lock. The delete path now callskill::release_the_lock(flows/lifecycle/delete.rs), the same sweep that a launch uses. Before, it printed advice to rundl <ws> killand continued to wait. A holder whose parent is alive is spared.LockWait,clients/devpod.rs). A holder that the first sweep spared can lose its parent later. A later sweep prints a line only when it signals something.SIGKILLafterSIGTERM. The handler sendsSIGTERMto thedevpod upgroup and waits up to 2 s for the group leader. Then it sendsSIGKILLto the group. Before, it sentSIGTERMand exited at once. Without the wait, the kernelSIGKILLfrom the first bullet would land right after theSIGTERM, anddevpod upwould get no time to stop cleanly.CHANGELOG.md,README.mdanddocs/cli.mddescribe the new behavior. The two public-api snapshots listends_with_this_process.Where this fits
How I checked it
Before → after, each test fails on
origin/mainand passes on this branch:devlaunch-runner/tests/parent_death.rs: start a parent that spawns an own-group child,SIGKILLthe parent. Before: the child lives on. After: the child is gone.dl/tests/up_blocked.rs:SIGKILLdlduringdevpod up. Before: the fakedevpod uplives on. After: it is gone. Other tests in this file cover a sweep that repeats, and a later sweep that finds a holder the first sweep spared.aid/tests/interactive.rsa_sigkilled_aid_takes_its_boot_down_and_the_token_with_it:SIGKILLaid. After: the boot child stops and removes its token.flows/lifecycle/tests.rsanddl/tests/lifecycle.rs: a delete behind an orphan sweeps it and reports the sweep. A delete behind a live build signals nothing. A delete that stays blocked sweeps again.dl/tests/interrupt.rs: Ctrl-C duringdevpod upgivesdevpodits 2 s grace, then the group getsSIGKILL.Each test that guards a branch also failed when I mutated the code that it guards.
Commands I ran on the host:
cargo fmt --checkandcargo clippy --locked --all-targets -- -D warnings: clean.cargo test --workspace --no-fail-fast: 2923 passed and 5 failed. The same 5 fail onorigin/main: 4 indevlaunch-coreflows::lifecycle::tests(for examplea_remote_that_cannot_be_reached_leaves_the_refusal_standing) anddl/tests/lifecycle.rs::a_commit_pushed_by_url_does_not_stop_rm. They look like a host git-remote setup problem.pixi run test: 834 passed, 7 skipped.prek run --from-ref origin/main --to-ref HEAD: clean.I did not compile for macOS. No Darwin target is installed here, and CI has no macOS job.
Merge danger
Door: two-way. A revert brings back the old behavior. No stored data or file format changes.
Blast radius: small. If the sweep is wrong, it can signal a
devpod upthat another livedlwaits on. The sweep spares a holder whose parent is alive, and a test covers that case. A Ctrl-C now takes up to 2 s longer, but only whendevpod upignoresSIGTERM. A normal Ctrl-C took 0.29 s in the test.Review notes
getppidrace branch, because a test cannot cause that race on demand.kill.rs:203says that the sweep covers this case, and that is not true. This gap was already onmain, and the new sweeps have the same gap.dl <ws> killends a wait behind a live build. That is false, and it is already onmain. This PR fixes the same text on the delete path only.6de25f9is the bump to 0.59.3, so a merge publishes the fix.🤖 Generated with Claude Code
Summary by Sourcery
Prevent workspace-locking processes from becoming orphaned and make launches and deletes recover automatically from abandoned lock holders.
Bug Fixes:
devpod upand background boot processes from outliving thedloraidprocess that started them.devpod upprocess groups receive a graceful termination period before being forcefully killed.Enhancements:
Build:
Documentation:
Tests: