Skip to content

Commit e92e185

Browse files
fix(watcher): isolate stale native watch generations
Keep retained watcher handles, callbacks, and shims from pausing, draining, closing, purging, or injecting into a live native-watch generation. Acknowledge obsolete aggregates so delivery cannot stall and cover long-lived raw listeners, stale handles, and generation replacement.
1 parent eb10e07 commit e92e185

2 files changed

Lines changed: 148 additions & 6 deletions

File tree

packages/rspack/src/NativeWatchFileSystem.ts

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,11 @@ export default class NativeWatchFileSystem implements WatchFileSystem {
169169
// Fresh shim per cycle (see field comment). Events are emitted to both the
170170
// long-lived `#events` (the `on`/`once` API) and this cycle's shim (the
171171
// `.watcher` surface).
172-
const watcher = new NativeWatcherShim((kind, path) =>
173-
this.#inner?.triggerEvent(kind, path),
174-
);
172+
const watcher = new NativeWatcherShim((kind, path) => {
173+
if (this.#watcher === watcher) {
174+
nativeWatcher.triggerEvent(kind, path);
175+
}
176+
});
175177
this.#watcher = watcher;
176178
let lastDrainedGeneration: number | undefined;
177179

@@ -181,6 +183,12 @@ export default class NativeWatchFileSystem implements WatchFileSystem {
181183
this.formatWatchDependencies(missing),
182184
BigInt(startTime),
183185
(err: Error | null, result) => {
186+
if (this.#watcher !== watcher) {
187+
if (!err) {
188+
nativeWatcher.acknowledgePendingEvents(result.generation);
189+
}
190+
return;
191+
}
184192
if (err) {
185193
callback(err, new Map(), new Map(), new Set(), new Set());
186194
return;
@@ -232,9 +240,10 @@ export default class NativeWatchFileSystem implements WatchFileSystem {
232240

233241
return {
234242
close: () => {
235-
if (this.#watcher === watcher) {
236-
this.#watcher = undefined;
243+
if (this.#watcher !== watcher) {
244+
return;
237245
}
246+
this.#watcher = undefined;
238247
if (this.#inner === nativeWatcher) {
239248
this.#inner = undefined;
240249
this.#isFirstWatch = true;
@@ -245,10 +254,20 @@ export default class NativeWatchFileSystem implements WatchFileSystem {
245254
},
246255

247256
pause: () => {
248-
nativeWatcher.pause();
257+
if (this.#watcher === watcher) {
258+
nativeWatcher.pause();
259+
}
249260
},
250261

251262
getInfo: () => {
263+
if (this.#watcher !== watcher) {
264+
return {
265+
changes: new Set(),
266+
removals: new Set(),
267+
fileTimeInfoEntries: new Map(),
268+
contextTimeInfoEntries: new Map(),
269+
};
270+
}
252271
const { changedFiles, removedFiles, generation } =
253272
nativeWatcher.takePendingEvents();
254273
lastDrainedGeneration = generation;

tests/rspack-test/NativeWatcherGeneration.test.js

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ const { rspack } = require("@rspack/core");
33
class FakeNativeWatcher {
44
acknowledgements = [];
55
pauses = 0;
6+
closes = 0;
7+
drains = 0;
8+
triggered = [];
69
pendingDrain = {
710
changedFiles: [],
811
removedFiles: [],
@@ -15,6 +18,7 @@ class FakeNativeWatcher {
1518
}
1619

1720
takePendingEvents() {
21+
this.drains++;
1822
return this.pendingDrain;
1923
}
2024

@@ -26,7 +30,12 @@ class FakeNativeWatcher {
2630
this.pauses++;
2731
}
2832

33+
triggerEvent(kind, path) {
34+
this.triggered.push([kind, path]);
35+
}
36+
2937
close() {
38+
this.closes++;
3039
return Promise.resolve();
3140
}
3241
}
@@ -154,4 +163,118 @@ describe("NativeWatchFileSystem aggregate generations", () => {
154163
nativeWatcher.onRaw({ kind: "change", path: "/after-close" });
155164
expect(fresh).toEqual(["/fresh"]);
156165
});
166+
167+
it("forwards concrete child events to long-lived watch-file-system listeners", () => {
168+
const events = [];
169+
const { nativeWatcher, watchFileSystem } = createWatcherHarness();
170+
watchFileSystem.on("change", (file, mtime) =>
171+
events.push(["change", file, mtime])
172+
);
173+
watchFileSystem.on("remove", file => events.push(["remove", file]));
174+
const staleRaw = nativeWatcher.onRaw;
175+
176+
const currentWatcher = watchFileSystem.watch(
177+
dependencies(),
178+
dependencies(),
179+
dependencies(),
180+
Date.now(),
181+
{},
182+
() => {},
183+
() => {}
184+
);
185+
186+
staleRaw({ kind: "change", path: "/src/stale.js" });
187+
staleRaw({ kind: "remove", path: "/src/stale.js" });
188+
nativeWatcher.onRaw({ kind: "change", path: "/src/created.js" });
189+
nativeWatcher.onRaw({ kind: "remove", path: "/src/created.js" });
190+
expect(events).toEqual([
191+
["change", "/src/created.js", expect.any(Number)],
192+
["remove", "/src/created.js"]
193+
]);
194+
195+
currentWatcher.close();
196+
nativeWatcher.onRaw({ kind: "change", path: "/src/after-close.js" });
197+
nativeWatcher.onRaw({ kind: "remove", path: "/src/after-close.js" });
198+
expect(events).toHaveLength(2);
199+
});
200+
201+
it("prevents stale watcher handles and callbacks from affecting the live generation", () => {
202+
const events = [];
203+
const {
204+
nativeWatcher,
205+
purged,
206+
callbackChanges,
207+
watcher: staleWatcher,
208+
watchFileSystem
209+
} = createWatcherHarness();
210+
watchFileSystem.on("aggregated", (changes, removals) =>
211+
events.push([changes, removals])
212+
);
213+
const staleAggregate = nativeWatcher.onAggregate;
214+
const staleShim = watchFileSystem.watcher;
215+
216+
const currentChanges = [];
217+
const currentWatcher = watchFileSystem.watch(
218+
dependencies(),
219+
dependencies(),
220+
dependencies(),
221+
Date.now(),
222+
{},
223+
(_error, _fileTimes, _contextTimes, changes) =>
224+
currentChanges.push(changes),
225+
() => {}
226+
);
227+
228+
nativeWatcher.pendingDrain = {
229+
changedFiles: ["/src/live.js"],
230+
removedFiles: [],
231+
generation: 2
232+
};
233+
staleWatcher.pause();
234+
expect(staleWatcher.getInfo()).toEqual({
235+
changes: new Set(),
236+
removals: new Set(),
237+
fileTimeInfoEntries: new Map(),
238+
contextTimeInfoEntries: new Map()
239+
});
240+
staleWatcher.close();
241+
staleShim._onChange("/src/stale.js");
242+
staleShim._onRemove("/src/stale.js");
243+
staleAggregate(null, {
244+
changedFiles: ["/src/stale.js"],
245+
removedFiles: [],
246+
generation: 1
247+
});
248+
249+
expect(nativeWatcher.pauses).toBe(0);
250+
expect(nativeWatcher.drains).toBe(0);
251+
expect(nativeWatcher.closes).toBe(0);
252+
expect(nativeWatcher.triggered).toEqual([]);
253+
expect(nativeWatcher.acknowledgements).toEqual([1]);
254+
expect(purged).toEqual([]);
255+
expect(callbackChanges).toEqual([]);
256+
expect(currentChanges).toEqual([]);
257+
expect(events).toEqual([]);
258+
259+
nativeWatcher.onAggregate(null, {
260+
changedFiles: ["/src/live.js"],
261+
removedFiles: [],
262+
generation: 2
263+
});
264+
expect(nativeWatcher.pauses).toBe(1);
265+
expect(nativeWatcher.acknowledgements).toEqual([1, 2]);
266+
expect(purged).toEqual(["/src/live.js"]);
267+
expect(callbackChanges).toEqual([]);
268+
expect(currentChanges).toEqual([new Set(["/src/live.js"])]);
269+
expect(events).toEqual([[new Set(["/src/live.js"]), new Set()]]);
270+
watchFileSystem.watcher._onChange("/src/live.js");
271+
watchFileSystem.watcher._onRemove("/src/live.js");
272+
expect(nativeWatcher.triggered).toEqual([
273+
["change", "/src/live.js"],
274+
["remove", "/src/live.js"]
275+
]);
276+
277+
currentWatcher.close();
278+
expect(nativeWatcher.closes).toBe(1);
279+
});
157280
});

0 commit comments

Comments
 (0)