Repository navigation
Commit e9a1f5c
fix(core): a stopped ObjectKernel removes its signal listeners and never exits the process (#22334)
Fixes #22286
Clause-②: no
## What was wrong
With `gracefulShutdown: true` (the default), the `ObjectKernel`
constructor called `registerShutdownSignals()`. That added one anonymous
`process.on(signal, ...)` listener each for SIGINT, SIGTERM and SIGQUIT
and kept no handle, so `shutdown()` could never remove them. A stopped
kernel's handler still ran `shutdown()`, which warned and returned, and
then called `safeExit(0)`. Every kernel a process built therefore left
three handlers, and each one exited the process on the next signal.
## What changes (`packages/core/src/kernel.ts`)
- `registerShutdownSignals()` stores each listener in a private
`signalListeners` map.
- `releaseShutdownSignals()` removes exactly those listeners and is
idempotent. It runs at every transition into `stopped`: `shutdown()`
completing, the shutdown-timeout branch, and a failed `bootstrap()`. A
stopped kernel holds no listener.
- `handleShutdownSignal()` exits the process only from the handler whose
signal started the drain, and only once. If the kernel is already
`stopping` or `stopped` when a signal arrives, the handler logs a
warning and does not exit, because whoever is stopping the kernel owns
what follows.
- The `gracefulShutdown` JSDoc states the listener lifetime. There is no
new option, export or accept-set change.
**Why the listeners are removed at `stopped`, not when the drain
begins.** The dispatch's suggested route removed them at the start of
`shutdown()` and on the first signal. That would regress the existing
duplicate-signal guard (`Shutdown already in progress, ignoring
SIGINT`). While a kernel drains, its listener absorbs a repeated signal,
for example a terminal Ctrl-C that reaches both the process group and a
forwarding parent. With no listener left, that second signal gets Node's
default action and kills the process mid-drain. So the listener stays
installed through `stopping` and is removed at `stopped`. Pin 6 below
covers this.
## Before / after
Measured with a probe that stubs `process.exit` and invokes the
listeners the kernels added, as a delivered signal would. Before =
`packages/core/src/kernel.ts` at `f2626c71db` (blob `57d2340c`); after =
this branch (blob `2307a7b5`). Counts are listeners added per signal
(SIGINT / SIGTERM / SIGQUIT, always equal).
| Scenario | Before | After |
|---|---|---|
| 5 kernels built, booted and stopped: listeners left | +5 | +0 |
| One SIGTERM with those 5 stopped kernels plus 1 live kernel:
`process.exit` calls | 6 × `exit(0)` | 1 × `exit(0)` |
| Listeners left after that SIGTERM | +6 | +0 |
| SIGTERM while the host's own `shutdown()` is draining: exits during
the drain | 1 × `exit(0)` | 0 |
| SIGINT, then SIGINT and SIGTERM again during that signal's drain:
exits | 1 × `exit(0)`, after the drain | 1 × `exit(0)`, after the drain
|
| Listeners installed during that drain / after it | +1 / +1 | +1 / +0 |
| Failed `bootstrap()`: listeners left | +1 | +0 |
| 11 kernels built and stopped: `MaxListenersExceededWarning` | 3 (one
per signal) | 0 |
| Two live default kernels, one SIGTERM: exits | 2 × `exit(0)` | 2 ×
`exit(0)` (unchanged, see Acceptance notes) |
| Family item: the host stops one of two `gracefulShutdown: false`
kernels and its teardown hangs past `shutdownTimeout` | `exit(1)`, other
kernel still `running` | unchanged |
## The family item: the teardown-timeout `process.exit(1)` is not folded
in
- **Reach, measured in process with a stubbed exit.** A host holding two
`gracefulShutdown: false` kernels (cloud's per-environment shape) stops
one of them. That kernel's teardown hangs past `shutdownTimeout`, and
`shutdown()` calls `process.exit(1)` while the other kernel is still
`running`.
- **Why it is not folded.** No rule in the code makes `gracefulShutdown:
false` mean "this kernel does not own the process". The only place that
says so is the docs page `content/docs/protocol/kernel/lifecycle.mdx`:
"ObjectStack ships the listeners only when `gracefulShutdown: true`;
otherwise it stays out of the process-management business". The code
pins the opposite. In `kernel.test.ts`, the test "still logs the timeout
and still forces exit(1) when shutdown genuinely times out" builds its
kernel with `gracefulShutdown: false` and asserts `exit(1)`. Folding the
exit under `gracefulShutdown` would reverse that landed pin and give an
existing option a second meaning. Any other fold needs a new option,
which widens the public surface. So the question goes to the maintainer
with a four-axis analysis (the os-dev report on the card), and this PR
leaves the timeout exit unchanged.
## Tests
New file `packages/core/src/kernel-signal-listeners.test.ts`, 9 tests.
`process.exit` is stubbed throughout, and listeners added during a test
are removed in `afterEach`.
1. Five kernels built and stopped leave the baseline listener count on
SIGINT, SIGTERM and SIGQUIT. Positive control: a live kernel holds
exactly one listener per signal.
2. One signal produces one exit with five stopped kernels and one live
kernel in the process.
3. to 5. Control, one test per signal: a running kernel still drains
(`kernel:shutdown`, then `destroy()`) and then exits once with code 0.
6. A repeated signal during a signal-driven drain is absorbed, and the
listener is still installed while the kernel is `stopping`. It starts no
second exit.
7. A signal that arrives while the host is stopping the kernel does not
exit the process.
8. A kernel whose bootstrap failed releases its listeners.
9. `gracefulShutdown: false` installs no listener.
**Reverse verification and ablations** (one-off, run at `74a027b369`).
The test imports `./kernel` relatively, so it reads source: no `dist/`
and no vitest alias are involved. Each leg restores `kernel.ts` with
`git checkout HEAD --` and proves the restore by blob hash (`2307a7b5`
equals HEAD) and an empty `git diff HEAD`.
- **Leg A**, `kernel.ts` restored to the base `f2626c71db` and the test
at HEAD: 8 failed, 1 passed (the `gracefulShutdown: false` control). The
one-signal pin reads `expected [ +0, +0, +0, +0, +0, +0 ] to deeply
equal [ +0 ]`.
- **Leg B**, removal dropped: the anchor `process.removeListener(signal,
listener);` is replaced with a no-op through
`scripts/ablation-replace.mjs` (anchor 1 → 0, blob `2307a7b5` →
`efc9b4e6`). 8 failed, 1 passed. The one-signal pin reads `expected {
SIGINT: 6, SIGTERM: 6, SIGQUIT: 6 } to deeply equal { SIGINT: 1,
SIGTERM: 1, SIGQUIT: 1 }`.
- **Leg C**, the `stopping`/`stopped` guard in `handleShutdownSignal()`
dropped (blob → `923bb1da`): exactly 1 failed, the host-stopping pin,
`expected [ +0 ] to deeply equal []`. This guard has its own pin, and
removing listeners alone does not cover it.
**Suites**, run at `87a131389e`. That is this branch with `origin/main`
`28bff18d0c` merged in; the merge touches neither `kernel.ts` nor the
new test, whose blobs stay `2307a7b5` and `f7c6469d`. The dependency
closure was rebuilt first.
- `@objectstack/core` local project: 80 files, 2208 tests passed. Repo
project: 3 files, 48 tests passed.
- `@objectstack/runtime` local project: 340 files, 4773 passed, 19
skipped. Repo project: 3 files, 751 passed. Both ran against
`@objectstack/core` built from this branch.
- `pnpm --filter @objectstack/core typecheck` passed (tsc, examples, and
`check:test-typecheck`: the test layer compiles and the ledger is
unchanged at 4 entries).
**Gates.** `node scripts/pm/dispatch-gates.mjs --commands` at
`87a131389e` derives 63 commands. All 63 ran and exited 0, and `--ran`
with the recorded exit codes reports 63 run, 0 NOT-MEASURED, 0 UNRUN.
Locally, `check-changeset-no-major` reports its clause-② LEVEL axis as
not applicable because there is no `pull_request` payload, so CI reads
this body for it. The CI-owned jobs (Test Core shards, Dogfood, Temporal
Conformance, Build Core, the workspace type-check lanes) were not run
locally.
## Acceptance notes
- **Two live kernels that both own the signals still exit twice.** Each
one drains and exits on the same signal, so in a real process the first
exit ends the other kernel's drain. Neither kernel is stopped, so this
falls outside the ruling. It is the same "who owns the process" question
as the family item. Carrier: that decision.
- **A kernel that is constructed and never bootstrapped keeps its
listeners.** `shutdown()` on an `idle` kernel still throws `Kernel not
running`, unchanged. Removing the listeners there would leave a kernel
that is bootstrapped later with no signal handling. Carrier: none.
- **A signal during a host-driven `shutdown()` is now logged and left to
the host,** where before it exited the process at once. `objectstack
serve` calls `shutdown()` itself only on the migrate-and-exit path,
which then calls `process.exit(0)`.
- **No other process-level listener.** `git grep` over
`packages/core/src` finds no `process.on`/`once`/`addListener` besides
these three signals: no `uncaughtException`, `unhandledRejection` or
`beforeExit`. Outside core, `driver-sqlite-wasm`'s `beforeExit` listener
already keeps its handle and removes it.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 8a995b8 commit e9a1f5c
3 files changed
Lines changed: 321 additions & 25 deletions
File tree
- .changeset
- packages/core/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
73 | 78 | | |
74 | 79 | | |
75 | 80 | | |
| |||
116 | 121 | | |
117 | 122 | | |
118 | 123 | | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
119 | 141 | | |
120 | 142 | | |
121 | 143 | | |
| |||
513 | 535 | | |
514 | 536 | | |
515 | 537 | | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
516 | 541 | | |
517 | 542 | | |
518 | 543 | | |
| |||
566 | 591 | | |
567 | 592 | | |
568 | 593 | | |
| 594 | + | |
569 | 595 | | |
570 | 596 | | |
571 | 597 | | |
| 598 | + | |
572 | 599 | | |
573 | 600 | | |
574 | 601 | | |
| |||
899 | 926 | | |
900 | 927 | | |
901 | 928 | | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
902 | 934 | | |
| 935 | + | |
903 | 936 | | |
904 | | - | |
905 | | - | |
906 | | - | |
907 | | - | |
908 | | - | |
909 | | - | |
910 | | - | |
911 | | - | |
912 | | - | |
913 | | - | |
914 | | - | |
915 | | - | |
916 | | - | |
917 | | - | |
918 | | - | |
919 | | - | |
920 | | - | |
921 | | - | |
922 | | - | |
923 | | - | |
924 | | - | |
925 | | - | |
926 | | - | |
927 | | - | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
928 | 987 | | |
929 | 988 | | |
930 | 989 | | |
| |||
0 commit comments