Skip to content

fix(cli): wait for the task tree's graceful shutdown on ctrl-c#2070

Closed
fengmk2 wants to merge 4 commits into
issue-2036-snapshot-reprofrom
issue-2036-ctrlc-wait-fix
Closed

fix(cli): wait for the task tree's graceful shutdown on ctrl-c#2070
fengmk2 wants to merge 4 commits into
issue-2036-snapshot-reprofrom
issue-2036-ctrlc-wait-fix

Conversation

@fengmk2

@fengmk2 fengmk2 commented Jul 6, 2026

Copy link
Copy Markdown
Member

Closes #2036

Builds on #2065 by @forehalo: this branch carries both of its commits as-is (the execute_with_terminal_guard integration with terminal-state restore, applied to both delegation call sites), with follow-up commits adjusting the signal handling on top.

Background: Ctrl+C is delivered by the terminal to the whole foreground process group, and the delegating global vp is the PTY session leader. It used to die of SIGINT within milliseconds, handing the terminal back to the shell (and SIGHUP'ing the session) while the task tree (vite dev server, managed node) was still shutting down. The dying tree's late terminal writes and mode restores then raced the shell's prompt probes (fish sends OSC 11, CSI 6n, and DA1 at every prompt draw), turning the terminal's replies into the garbage input from the issue.

Adjustments on top of #2065:

  • Interrupt swallowing moves from the guard's SIG_IGN sigaction into the wait loop as a signal handler. SIG_IGN survives exec into the entire child tree, so any task without its own SIGINT handler would become uninterruptible (node launders inherited dispositions at startup, which masks this for the delegation path today, but the guard is shared code and the binding call site spawns other children). A handler resets to default on exec and leaves children untouched, and also works when stdin is not a terminal, where the guard never installs.
  • A second Ctrl+C force-kills the child, so a task that ignores the first interrupt cannot make vp unstoppable. Verified against the SIG_IGN variant: a stuck task left vp hanging until externally killed.
  • Children killed by a signal now exit with the conventional 128 + signal code (130 for SIGINT, 137 for the force-kill) instead of a generic 1.
  • Windows gets the same swallow-and-escalate loop via the console Ctrl+C handler.

The run_ctrlc_teardown snapshot from #2069 flips to task completed its graceful shutdown and now guards the waiting behavior; a second case pins the escalation (task ignoring the interrupt, force-killed by the second Ctrl+C, exit 137). Verified end to end by driving real fish 4.8 in a scripted PTY: the stray escape sequences no longer appear on the prompt after killing vp run dev.

Known residual edge (follow-up, vite-task side): a shell-wrapped script line whose intermediate shell dies of SIGINT can still orphan grandchildren, since vite_task waits only for its direct child; waiting on the task's process group would close that.

fengmk2 commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label auto-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@fengmk2 fengmk2 added the preview-build Publish this PR's commits to the registry bridge as preview builds label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Registry bridge build (060115f)

This commit build is published to the registry bridge, which serves these as ordinary npm versions (every other package proxies to npmjs):

Package Version
vite-plus 0.0.0-commit.060115f340ebbeed547dd5eff887b0d23f8baee4
@voidzero-dev/vite-plus-core 0.0.0-commit.060115f340ebbeed547dd5eff887b0d23f8baee4

Install the Vite+ CLI built from this commit, then migrate a project:

# macOS / Linux
curl -fsSL https://vite.plus | VP_PR_VERSION=2070 bash
# Windows (PowerShell)
$env:VP_PR_VERSION="2070"; irm https://vite.plus/ps1 | iex

After installing, upgrade the current project's vite-plus to this test build with:

vp migrate

Or point your package manager at the bridge registry https://registry-bridge.viteplus.dev/:

Package manager Registry config
npm / pnpm / Bun .npmrc: registry=https://registry-bridge.viteplus.dev/
Yarn (v2+) .yarnrc.yml: npmRegistryServer: "https://registry-bridge.viteplus.dev/"

Then pin the build (vite aliases to vite-plus-core; pnpm can use a catalog, npm an overrides entry):

{
  "devDependencies": {
    "vite-plus": "0.0.0-commit.060115f340ebbeed547dd5eff887b0d23f8baee4",
    "vite": "npm:@voidzero-dev/vite-plus-core@0.0.0-commit.060115f340ebbeed547dd5eff887b0d23f8baee4"
  }
}

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🐳 Docker preview image

Built from this PR's registry bridge build:

Image Compressed size
ghcr.io/voidzero-dev/vite-plus:pr-2070 240MB
# remove any stale local copy from a previous run, then pull fresh
docker rmi ghcr.io/voidzero-dev/vite-plus:pr-2070 2>/dev/null; docker pull ghcr.io/voidzero-dev/vite-plus:pr-2070

Quick check:

docker run --rm ghcr.io/voidzero-dev/vite-plus:pr-2070 vp --version

See docs/guide/docker.md for usage.

fengmk2 added 2 commits July 7, 2026 01:08
…rl-c

Builds on the terminal-guard approach from #2065 with three adjustments:

Swallowing moves out of the guard's sigaction and into the wait loop as a
signal handler. SIG_IGN survives exec into the entire child tree, so any
task without its own SIGINT handler (a plain script, a shell) would become
uninterruptible; handler dispositions reset to default on exec and leave
children untouched. It also applies when stdin is not a terminal, where
the guard's SIG_IGN never installed at all.

A second Ctrl+C force-kills the child, so a task that ignores the first
interrupt cannot make vp unstoppable. Without it, killing a stuck task
requires an external signal.

Children killed by a signal now exit with the conventional 128 + signal
code (130 for SIGINT) instead of a generic failure.

Refs #2036
…scalation

The run_ctrlc_teardown snapshot flips from "task was torn down before its
graceful shutdown finished" to "task completed its graceful shutdown" and
now guards the waiting behavior. A second case covers the escalation: a
task that ignores the interrupt (vpt report-orphan-on-ctrlc
--ignore-interrupt) keeps vp waiting after the first Ctrl+C and is
force-killed by the second, with vp exiting 137 and the surviving watcher
recording the unfinished shutdown. The "interrupted" milestone between the
two Ctrl+C presses keeps them from coalescing into a single signal.
@fengmk2 fengmk2 force-pushed the issue-2036-ctrlc-wait-fix branch from 7b79f87 to 20d6606 Compare July 6, 2026 17:09
@fengmk2

fengmk2 commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Consolidated into #2065: the fix commits (including the adjustments from this PR) have been pushed onto that branch so the fix lands under the original contributor's PR.

@fengmk2 fengmk2 closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview-build Publish this PR's commits to the registry bridge as preview builds

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants