Skip to content

fix(cmd): keep app context alive for graceful pack shutdown#345

Open
skhaz wants to merge 1 commit into
mainfrom
fix/pack-shutdown-cancel
Open

fix(cmd): keep app context alive for graceful pack shutdown#345
skhaz wants to merge 1 commit into
mainfrom
fix/pack-shutdown-cancel

Conversation

@skhaz

@skhaz skhaz commented Jun 15, 2026

Copy link
Copy Markdown
Member

Why

wippy run <hub-module> (and .wapp pack files) logged
failed to stop controllers during shutdown: supervisor is stopped: context canceled
on exit and abandoned services instead of stopping them gracefully.

What

The pack runner (runPackEntries) passed the appCtx cancel func as the
first-signal callback, so the shutdown signal cancelled the context the
supervisor and all its controllers derive from before shutdown.Perform
ran the graceful stop — making every controller.Stop() short-circuit. Pass
nil instead, matching the server path in run.go. The existing
defer cancel() still tears down appCtx after shutdown completes.

Testing

  • make test — 292 packages, 0 failures; golangci-lint v2.8.0 — 0 issues.
  • New regression test TestRunPackEntries_GracefulShutdownStopsRunningService
    (-race, -count=5, no flakes): fails when reverted to cancel, passes with nil.
  • Gremlins mutation on run_pack.go: 36 killed, 0 lived, 100% efficacy.
  • Manual repro on three hub modules (kickside/kickside, keeper/keeper,
    userspace/users): shutdown "supervisor is stopped" errors 3 → 0; services
    now log clean graceful stop.

@skhaz skhaz requested a review from wolfy-j June 15, 2026 14:23
Comment thread cmd/wippy/cmd/run_pack.go
}

waitForShutdownSignal(sigChan, logger, cancel)
waitForShutdownSignal(sigChan, logger, nil)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Don't cancel appCtx on the signal — it pre-empts the graceful stop. Let shutdown.Perform run first; defer cancel() cleans up after.

Why:
Running a hub module or pack (`wippy run <module>`) logged
"failed to stop controllers during shutdown: supervisor is stopped:
context canceled" and abandoned services instead of stopping them
gracefully on exit.

What:
runPackEntries passed the appCtx cancel func as the first-signal
callback, so the shutdown signal cancelled the context the supervisor
and all its controllers derive from before shutdown.Perform ran the
graceful stop, making every controller.Stop() short-circuit. Pass nil
instead, matching the server path in run.go; the existing deferred
cancel still tears down appCtx after shutdown completes. Adds a
regression test that registers a running service and asserts graceful
stop on signal.
@skhaz skhaz force-pushed the fix/pack-shutdown-cancel branch from 54cf327 to c5b4dbb Compare June 15, 2026 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants