From 9744c520753b1f4537779a1991fd1db7716b6617 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 17 Aug 2026 15:46:26 +0200 Subject: [PATCH 1/2] fix(project): Cancel pending settle timer before watcher recovery On Windows, a settle timer callback firing into a closed ReadDirectoryChangesW handle after recovery causes an access violation (0xC0000005). Cancel any pending timer before tearing down the subscriptions; it is re-armed by the first event on the new set. Extract the timer-cancel and subscription-drain logic shared by #recoverWatcher and destroy into #cancelSettleTimer and #drainSubscriptions helpers to remove the duplication. --- .../lib/graph/ProjectDefinitionWatcher.js | 30 ++++++++++----- .../lib/graph/ProjectDefinitionWatcher.js | 38 +++++++++++++++++++ 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/packages/project/lib/graph/ProjectDefinitionWatcher.js b/packages/project/lib/graph/ProjectDefinitionWatcher.js index d9a13549128..ad710ecda01 100644 --- a/packages/project/lib/graph/ProjectDefinitionWatcher.js +++ b/packages/project/lib/graph/ProjectDefinitionWatcher.js @@ -230,13 +230,13 @@ class ProjectDefinitionWatcher extends EventEmitter { } try { + this.#cancelSettleTimer(); + // Tear down the current subscriptions and re-subscribe the same watch set. The include // set (#watchedFiles / #watchDirs) is unchanged; only the OS-level handles are renewed. // Teardown failures are ignored here: the handles are discarded either way, and the // re-subscribe below is what decides whether recovery succeeded. - const subscriptions = this.#subscriptions; - this.#subscriptions = []; - await drainSubscriptions(subscriptions); + await this.#drainSubscriptions(); if (this.#destroyed) { return; } @@ -259,19 +259,29 @@ class ProjectDefinitionWatcher extends EventEmitter { */ async destroy() { this.#destroyed = true; + this.#cancelSettleTimer(); + const failures = await this.#drainSubscriptions(); + if (failures.length) { + const err = new AggregateError(failures, "Failed to unsubscribe one or more definition watchers"); + this.emit("error", err); + } + } + + // Cancels a pending settle timer, if any. Safe to call when no timer is armed. + #cancelSettleTimer() { if (this.#settleTimer) { clearTimeout(this.#settleTimer); this.#settleTimer = null; } - // Drain the subscriptions list first so a second destroy() is a no-op and a partial failure - // cannot leave stale handles to be unsubscribed twice. + } + + // Snapshots and clears the subscriptions list before draining it, so a second drain (a second + // destroy(), or a destroy() racing recovery) is a no-op and a partial failure cannot leave stale + // handles to be unsubscribed twice. Returns the unsubscribe failures for callers that report them. + async #drainSubscriptions() { const subscriptions = this.#subscriptions; this.#subscriptions = []; - const failures = await drainSubscriptions(subscriptions); - if (failures.length) { - const err = new AggregateError(failures, "Failed to unsubscribe one or more definition watchers"); - this.emit("error", err); - } + return drainSubscriptions(subscriptions); } } diff --git a/packages/project/test/lib/graph/ProjectDefinitionWatcher.js b/packages/project/test/lib/graph/ProjectDefinitionWatcher.js index ebf385a03df..15469eb36e8 100644 --- a/packages/project/test/lib/graph/ProjectDefinitionWatcher.js +++ b/packages/project/test/lib/graph/ProjectDefinitionWatcher.js @@ -454,6 +454,44 @@ test.serial("recovery: a watcher error tears down and re-subscribes", async (t) await watcher.destroy(); }); +test.serial("recovery: pending settle timer is cancelled before teardown", async (t) => { + const sub1 = createMockSubscription(); + const sub2 = createMockSubscription(); + let cb; + subscribeStub.onFirstCall().callsFake(async (_dir, callback) => { + cb = callback; + return sub1; + }); + subscribeStub.onSecondCall().resolves(sub2); + + const graph = createGraph({name: "root", rootPath: fixturePath("/app")}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + const ui5YamlPath = fixtureFile("/app", "ui5.yaml"); + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + + const clock = sinon.useFakeTimers(); + // Open a burst so the settle timer is armed. + cb(null, [{type: "update", path: ui5YamlPath}]); + clock.tick(100); + + // Watcher error fires while the timer is still pending. + cb(new Error("Failed to read changes")); + + // Restore real timers before awaiting recovery so async callbacks can proceed. + clock.restore(); + await new Promise((resolve) => setImmediate(resolve)); + + // The old subscription is torn down and a new one created. + t.true(sub1.unsubscribe.calledOnce, "old subscription torn down"); + t.is(subscribeStub.callCount, 2, "re-subscribed after recovery"); + + t.is(emitted.length, 0, "cancelled settle timer does not fire into the closed watcher handle"); + + await watcher.destroy(); +}); + test.serial("recovery: loop protection escalates to error after the max attempts", async (t) => { const subs = []; subscribeStub.callsFake(async () => { From d3f5cdb560d74e9a991a67dffe2163f317755f36 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 19 Aug 2026 10:51:32 +0200 Subject: [PATCH 2/2] fix(server): Serialize destroy() teardown against in-flight swap via exclusive lock Introduce #lockTail / #runExclusive so destroy()'s field teardown and #swap never run concurrently. destroy() fires its synchronous head (state flip, abort, timer/relay cleanup) immediately, then queues teardown behind any in-flight swap; the swap therefore always finishes adopting #stack / #definitionWatcher before destroy reads and nulls them, closing the race that orphaned a @parcel/watcher subscription and the node:sqlite database on Windows. Adds two regression tests: one for the concurrent-swap leak and one confirming destroy() unblocks promptly when a recovery is parked in the settle wait. --- packages/server/lib/serve/Supervisor.js | 79 +++++++++++------ .../test/lib/server/serve/Supervisor.js | 85 +++++++++++++++++++ 2 files changed, 140 insertions(+), 24 deletions(-) diff --git a/packages/server/lib/serve/Supervisor.js b/packages/server/lib/serve/Supervisor.js index af4834aa33a..6f0249d65d6 100644 --- a/packages/server/lib/serve/Supervisor.js +++ b/packages/server/lib/serve/Supervisor.js @@ -125,6 +125,15 @@ class Supervisor extends EventEmitter { #recoveryTimer = null; #destroyAbortController = new AbortController(); + // Serializes the swap body against destroy()'s teardown so the two never run concurrently. Both + // #swap() (via reinitialize) and destroy()'s field teardown acquire it; each waits out the other. + // This is what lets #swap read/mutate #stack and #definitionWatcher across its awaits without a + // concurrent destroy() tearing them down mid-flight — the source of the orphaned native handles + // (a @parcel/watcher subscription, the node:sqlite database) that kept the process alive on Windows. + // Distinct from #reinitInProgress, which only collapses overlapping reinitialize() calls; destroy() + // acquires this lock directly, never through reinitialize(), so the #init error path cannot deadlock. + #lockTail = Promise.resolve(); + // Stable reference handed to every stack buildApp() builds. Closes over the supervisor instance // (not a per-stack value), so the surviving stack's serveBuildError reads the current // #degradedError on each request even though it was assembled before the failed swap. @@ -169,6 +178,16 @@ class Supervisor extends EventEmitter { buildServer.resumeReaders(); } + // Runs `fn` only after every previously-queued exclusive operation has settled, serializing the + // swap body against destroy teardown. The stored tail swallows the outcome so a rejected `fn` (a + // failed swap) cannot wedge the lock for the next waiter; the returned promise still surfaces `fn`'s + // real result or rejection to its own caller. + #runExclusive(fn) { + const run = this.#lockTail.then(fn, fn); + this.#lockTail = run.then(() => {}, () => {}); + return run; + } + constructor(config, error, graphFactory, projectWatcher) { super(); this.#config = config; @@ -289,6 +308,12 @@ class Supervisor extends EventEmitter { // Suspending rejects those requests fast instead. Reads #stack.buildServer live, so it always // targets the current stack; the suspend is lifted via #liftSuspend on both #swap outcomes. watcher.on("definitionChanging", () => { + // A watcher can still emit between destroy()'s synchronous state flip and its teardown + // awaiting the watcher's own destroy(). Bail: teardown has nulled (or is about to null) + // #stack, and the suspend/budget work below is pointless on a server being torn down. + if (this.#state === STATE.DESTROYED) { + return; + } // A real definition change supersedes any pending self-scheduled recovery and restores a // full recovery budget: the user just acted, so the next attempt should not be denied by an // allowance spent on the previous branch. @@ -350,7 +375,7 @@ class Supervisor extends EventEmitter { try { do { this.#reinitQueued = false; - await this.#swap(); + await this.#runExclusive(() => this.#swap()); } while (this.#reinitQueued && this.#state !== STATE.DESTROYED); } finally { this.#reinitInProgress = false; @@ -502,11 +527,6 @@ class Supervisor extends EventEmitter { this.#scheduleDegradedRecovery(); return; } - if (this.#state === STATE.DESTROYED) { - // Destroyed while building: discard the new stack instead of adopting it. - await newStack.buildServer.destroy(); - return; - } // Swap: retarget the dispatcher, move live-reload to the new BuildServer, notify clients. this.#setState(STATE.HEALTHY); this.#stack = newStack; @@ -521,7 +541,9 @@ class Supervisor extends EventEmitter { this.#sourcesChangedRelay.emit("sourcesChanged"); // Re-target the definition watcher to the new graph: the project set or their roots may // have changed. A create failure here must not crash the swap: keep serving and log, so the - // old watcher keeps driving re-inits. + // old watcher keeps driving re-inits. destroy() cannot interleave here: it acquires the same + // exclusive lock this swap holds, so its teardown runs only after this swap returns and then + // tears down whatever this swap adopted as #stack / #definitionWatcher. const oldWatcher = this.#definitionWatcher; this.#definitionWatcher = null; try { @@ -551,17 +573,16 @@ class Supervisor extends EventEmitter { * @returns {Promise} Resolves once teardown completes */ async destroy() { - // Move to the terminal state synchronously, before the first await, so an in-flight #swap or a - // late definitionChanged sees DESTROYED at its next guard and adopts nothing. + // Synchronous head: runs before any await and before the lock is acquired, so an in-flight + // #swap or a late definitionChanged/recovery-timer sees DESTROYED at its next guard, and the + // abort unblocks a recovery settle wait immediately rather than after its full window. this.#setState(STATE.DESTROYED); this.#destroyAbortController.abort(); - // Stop the definition watcher early so a late event cannot start a re-init mid-teardown. - // The reinitialize() DESTROYED guard already no-ops such an event; this is defensive. - const definitionWatcher = this.#definitionWatcher; - this.#definitionWatcher = null; + this.#clearRecoveryTimer(); this.#liveReloadHandle?.close(); this.#detachRelay(); - this.#clearRecoveryTimer(); + // Stop accepting new requests now, before waiting out any in-flight swap. Awaited last so the + // returned promise resolves only once the socket is fully closed. const httpClosed = new Promise((resolve) => { if (!this.#httpServer) { resolve(); @@ -569,16 +590,26 @@ class Supervisor extends EventEmitter { } this.#httpServer.close(() => resolve()); }); - try { - await definitionWatcher?.destroy(); - } catch (err) { - log.verbose(`Error while destroying definition watcher: ${err?.message ?? err}`); - } - try { - await this.#stack?.buildServer.destroy(); - } catch (err) { - log.verbose(`Error while destroying BuildServer: ${err?.message ?? err}`); - } + + // Teardown of the swappable fields runs under the same exclusive lock as #swap, so it never + // races a swap mid-flight. By the time it runs, any in-flight swap has settled and + // #definitionWatcher / #stack point at whatever that swap adopted — read once, torn down once. + await this.#runExclusive(async () => { + const definitionWatcher = this.#definitionWatcher; + this.#definitionWatcher = null; + const stack = this.#stack; + this.#stack = null; + try { + await definitionWatcher?.destroy(); + } catch (err) { + log.verbose(`Error while destroying definition watcher: ${err?.message ?? err}`); + } + try { + await stack?.buildServer.destroy(); + } catch (err) { + log.verbose(`Error while destroying BuildServer: ${err?.message ?? err}`); + } + }); await httpClosed; } } diff --git a/packages/server/test/lib/server/serve/Supervisor.js b/packages/server/test/lib/server/serve/Supervisor.js index 55b9dbe70d9..54484e002ba 100644 --- a/packages/server/test/lib/server/serve/Supervisor.js +++ b/packages/server/test/lib/server/serve/Supervisor.js @@ -419,6 +419,91 @@ test("destroy() closes the socket even when BuildServer.destroy() rejects", asyn t.true(httpServer.close.calledOnce, "socket is closed despite the BuildServer destroy rejection"); }); +test("destroy() during a swap serializes behind it and leaks no watcher or stack", async (t) => { + // Regression for the Windows `Failed to exit` leak: destroy() used to tear down #definitionWatcher + // and #stack while an in-flight #swap was still mutating them, orphaning a freshly-armed watcher + // (and the old stack's native handles). Serializing #swap and destroy teardown through one lock + // means teardown runs only after the swap settles, then owns whatever the swap adopted. Here + // destroy() is fired (not awaited) while the swap is parked mid-build, so its synchronous head runs + // during the swap and its teardown queues behind it. + const stack1 = createStack(); + const stack2 = createStack(); + const createdWatchers = []; + const buildGate = Promise.withResolvers(); + const ref = {}; + let buildCalls = 0; + const {mocks, projectWatcher} = createMocks({ + buildAppImpl: async () => { + buildCalls++; + if (buildCalls === 1) { + return stack1; // initial build + } + // Park the swap's build so destroy() can land while the swap is in flight. + ref.destroyPromise = ref.supervisor.destroy(); + await buildGate.promise; + return stack2; + }, + definitionWatcherCreate: async () => { + const watcher = new EventEmitter(); + watcher.destroy = sinon.stub().resolves(); + createdWatchers.push(watcher); + return watcher; + }, + }); + const graphFactory = sinon.stub().resolves({}); + const {default: Supervisor} = await importSupervisor(mocks, projectWatcher); + + ref.supervisor = await Supervisor.create({}, baseConfig, undefined, graphFactory); + const reinit = ref.supervisor.reinitialize(); + // Let the swap reach its parked build and fire destroy()'s synchronous head, then release it. + await waitFor(() => Boolean(ref.destroyPromise)); + buildGate.resolve(); + await reinit; + await ref.destroyPromise; + + // The swap adopted stack2 + a re-targeted watcher; destroy's teardown then owned and released them, + // and the swap itself tore down the old stack + old watcher. Nothing is left live. + t.true(createdWatchers.every((w) => w.destroy.calledOnce), "every armed watcher is torn down, none leaked"); + t.true(stack1.buildServer.destroy.calledOnce, "the old stack is torn down"); + t.true(stack2.buildServer.destroy.calledOnce, "the stack adopted mid-swap is torn down, not leaked"); +}); + +test("destroy() during a recovery settle aborts the wait instead of blocking on it", async (t) => { + // destroy()'s synchronous head aborts the shared AbortController before it acquires the teardown + // lock. A degraded recovery parked in #waitForProjectGraphSettled must observe that abort and + // reject promptly (code ABORT_ERR), so destroy() does not block for the full settle window. + const stack1 = createStack(); + let buildCalls = 0; + const graph = createGraph(["/app"]); + const graphFactory = sinon.stub().resolves(graph); + const {mocks, projectWatcher, waitForProjectGraphSettled} = createMocks({ + buildAppImpl: async () => { + buildCalls++; + if (buildCalls === 1) { + return stack1; // initial build + } + throw new Error("invalid ui5.yaml"); // first reinit fails -> degraded, recovery follows + }, + }); + // A settle that honors the signal: it resolves only when the signal aborts, and then rejects with + // ABORT_ERR, mirroring the real waitForProjectGraphSettled. + waitForProjectGraphSettled.callsFake((graphs, {signal}) => new Promise((resolve, reject) => { + signal.addEventListener("abort", + () => reject(Object.assign(new Error("aborted"), {code: "ABORT_ERR"})), {once: true}); + })); + const {default: Supervisor} = await importSupervisor(mocks, projectWatcher); + const supervisor = await Supervisor.create(graph, baseConfig, undefined, graphFactory); + + await supervisor.reinitialize(); + t.is(buildCalls, 2, "the first reinitialize failed and left the stack degraded"); + + // Drive the recovery swap so it parks in the settle wait, then destroy() while it is parked. + const recovery = supervisor.reinitialize(); + await waitFor(() => waitForProjectGraphSettled.called); + await t.notThrowsAsync(supervisor.destroy(), "destroy() resolves without waiting the settle window"); + await recovery; +}); + test("reinitialize() warns and no-ops when no graphFactory was provided", async (t) => { const stack = createStack(); const {mocks, projectWatcher, buildApp} = createMocks({stacks: [stack]});