Skip to content

chore(dev-stack): drop the two kill calls that never matched anything - #209

Merged
gabitoesmiapodo merged 5 commits into
mainfrom
fix/188-dev-stack-down
Sep 14, 2026
Merged

gabitoesmiapodo merged 5 commits into
mainfrom
fix/188-dev-stack-down

Conversation

@gabitoesmiapodo

Copy link
Copy Markdown
Collaborator

Summary

Closes #188

The issue reports that down leaves the dApp dev server holding 3012. Running the full stack showed the port is freed. What it did find is two kill calls that reach no process at all.

Changes

  • down no longer runs a pkill pattern that matched nothing
  • stop_pidfile no longer runs a pkill -P that never reached vite
  • The comment on the surviving kill says what actually stops the dev server

Deviations

Acceptance criteria

  • down frees 3010-3012 and its port report prints (all free)
  • No kill call in the script targets a process that cannot exist
  • Behaviour is identical before and after

Test plan

Automated tests

None. No test runner covers scripts/dev-stack.sh.

Manual verification

  1. Check out this branch and run ./scripts/dev-stack.sh docker-up
  2. Run ./scripts/dev-stack.sh up and wait for "Stack is up"
  3. Open http://localhost:3012 and confirm the dApp loads
  4. Run ./scripts/dev-stack.sh down
  5. Expect its port report to print (all free)
  6. Run lsof -nP -iTCP:3010-3012 -sTCP:LISTEN and expect no output

Breaking changes

None.

Checklist

  • Self-reviewed my own diff
  • Tests added or updated
  • Docs updated (if applicable)
  • No unrelated changes bundled in

Screenshots

None.

`down` carried two fallbacks that looked like they stopped the dApp dev
server. Neither ever ran against a real process:

- `pkill -f "vite --host localhost --port 3012"` matched nothing. The
  real argv is `node .../vite.js` with no flags, because host, port and
  strictPort are set in `dapp/frontend/vite.config.ts`.
- `pkill -P "$pid"` signals only direct children, and vite sits two
  levels down: `pnpm run app:dev` spawns `pnpm -C dapp/frontend run
  dev`, which spawns vite.

Removing them changes no behaviour. The plain `kill "$pid"` already
stops the whole chain, because pnpm forwards SIGTERM to what it spawned.
The comment above it claimed the code killed a process group, which it
never did, so it now says what actually happens.

Checked by running the script end to end: docker-up, up, down. The stack
came up with 3010 and 3012 both answering, and after `down` nothing held
3010-3013 and no vite or pnpm process was left behind.

Refs #188
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
demo.canton-dappbooster Ready Ready Preview Sep 14, 2026 6:15pm UTC
docs.canton-dappbooster Ready Ready Preview Sep 14, 2026 6:15pm UTC

Request Review

@fernandomg fernandomg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been facing the same problem during local runs, and reached a different solution.

CC Report

The premise doesn't hold: killing the pid on file doesn't reach vite. The pidfile holds the outer pnpm run app:dev; vite is two levels down (pnpm run app:devpnpm -C dapp/frontend run devvite.js). Same shape, no ports involved:

{ "scripts": { "outer": "pnpm run inner", "inner": "node -e \"setInterval(()=>{},1000)\"" } }
nohup pnpm run outer & PID=$!; sleep 3; kill $PID; sleep 2; pgrep -f setInterval   # still running

The grandchild survives under pnpm 11.24.0 (our pin) and 12.3.4. So down reaches less after this PR than before — the removed pkill -P at least took the direct child.

Agreed on the pkill -f "vite --host …" line: vite's argv is …/vite/bin/vite.js --host …, so it never matched. Removing it is right.

A down that works regardless of the process tree kills by port instead: lsof -t -iTCP:3012 -sTCP:LISTEN, filtered to listeners whose cwd is this checkout so another checkout's stack is left alone.

The pidfile kill handles the normal case, but nothing covered a listener
no pidfile knows about: a crashed `up`, or a dev server someone started by
hand. `down` then reported the port busy and left the user to find the pid.

- `stop_port` looks up listeners with `lsof -t` and kills them
- It only kills a pid whose cwd is inside this checkout, so another clone
  running its own stack is untouched
- `down` calls it for 3010 and 3012 after the two `stop_pidfile` calls

Checked on a full run: with the stack up it prints nothing, because the
pidfile kills already freed both ports. With an untracked dev server on
3012 it frees the port. With a listener whose cwd is outside the checkout
it leaves the process alone and the port report shows 3012 still in use.
@gabitoesmiapodo

Copy link
Copy Markdown
Collaborator Author

@fernandomg

My CC's answer to this...

Ran the full loop through scripts/dev-stack.sh on this branch before changing anything.

The kill already reaches Vite

down does free 3012, and the single kill does reach vite.

With the stack up, the pidfile held the outer pnpm run app:dev, reparented to init:

78283     1 node .../pnpm run app:dev
78306 78283 node .../pnpm -C dapp/frontend run dev
78332 78306 node .../vite/bin/vite.js

vite.js held 3012 and curl http://localhost:3012/ returned 200. After ./scripts/dev-stack.sh down:

  • port report printed (all free)
  • lsof -nP -iTCP:3010-3012 -sTCP:LISTEN returned nothing
  • no app:dev, no frontend run dev, no vite.js, no canton-wallet-service left

Also ran your minimal outer / inner / setInterval case on pnpm 11.24.0 and node 24.20.0. The grandchild died on kill $PID. So pnpm forwards the signal down both levels here.

The pkill -P "$pid" this PR removes would not have helped either way: it reaches the direct child, which is the middle pnpm, and vite is one level below that.

What I took from your suggestion

Killing by port is worth having, just not for the reported symptom: it covers the case no pidfile can: a crashed up, or a dev server someone started by hand. Before this, down reported the port busy and left you to find the pid.

Added in 4d17826

down calls it for 3010 and 3012 after the two stop_pidfile calls.

One detail worth knowing: the cwd match has to be a prefix. wallet-service runs from the repo root, vite from dapp/frontend. A plain = against $ROOT_DIR would skip vite and the filter would never fire.

Three cases checked on a full up / down:

  1. Normal run: prints nothing, because the pidfile kills already freed both ports.
  2. Untracked dev server on 3012, no pidfile: Freeing port 3012 from a stray dApp dev server (pid 78625), then (all free).
  3. Listener on 3012 whose cwd is outside the checkout: left alone, still serving, and the port report shows 3012 in use.

shellcheck is clean. Teardown verified: no containers, no survivors, 3010-3012 free.

@gabitoesmiapodo
gabitoesmiapodo merged commit 2b6dfee into main Sep 14, 2026
9 checks passed
@gabitoesmiapodo
gabitoesmiapodo deleted the fix/188-dev-stack-down branch September 14, 2026 18:59
@github-project-automation github-project-automation Bot moved this from In review to Done in Canton - dAppBooster (#390) Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

dev-stack.sh down leaves the dApp dev server holding port 3012

2 participants