Skip to content

Commit 6699443

Browse files
committed
Revert "fix(gxt-backend): drain cross-test classic-reactor leaks at fire time"
This reverts commit 276a1a4.
1 parent 276a1a4 commit 6699443

2 files changed

Lines changed: 45 additions & 53 deletions

File tree

index.html

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,9 @@
142142
} catch (_e) {
143143
/* ignore */
144144
}
145-
// Leak diagnostics flag — kept off by default. Toggle via URL
146-
// (?gxtLeakDebug=true) only when reproducing the cross-test leak.
145+
// TEMPORARY: force-enable leak diagnostics for CI debugging.
146+
// Remove this line once the cumulative-state failures are diagnosed.
147+
globalThis.__GXT_LEAK_DEBUG__ = true;
147148

148149
// Global operation counter to detect infinite synchronous loops
149150
// Any tight loop will increment this. When it exceeds the limit,

packages/@ember/-internals/gxt-backend/validator.ts

Lines changed: 42 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -648,28 +648,21 @@ export function touchClassicBridge(): void {
648648
// invoked synchronously at the end of dirtyTagFor after the classic tag
649649
// has been updated but before the runloop flush. Registrants are expected
650650
// to be idempotent and cheap.
651-
// Each reactor is tagged at registration with the QUnit test it was
652-
// created in. On fire, reactors registered in a different test than
653-
// the one currently executing are unconditionally invalid (a real-app
654-
// reactor could never legitimately survive the test it was created
655-
// for) and we self-unsubscribe before invoking them. This replaces a
656-
// previous heuristic — a "_disconnectedTicks > 4" tick counter on the
657-
// target element — that proved unreliable: targetElement is often the
658-
// long-lived #qunit-fixture (which never disconnects between tests),
659-
// and routing tests reattach LinkTo elements between tests, resetting
660-
// the counter before it can trip. The diagnostic that confirmed the
661-
// leak path lives behind `__GXT_LEAK_DEBUG__`; the fix is on by
662-
// default. Tagging is always-on but cheap (one WeakMap put).
651+
// === LEAK DIAGNOSTICS ===
652+
// Each reactor is tagged with metadata at registration time so we can
653+
// detect cross-test leakage: a reactor registered during test A that
654+
// fires during test B is by definition a leak. Enabled via the global
655+
// flag `__GXT_LEAK_DEBUG__` (set in index.html or via URL param).
663656
interface ReactorMeta {
664657
id: number;
665658
source: string;
666659
registeredAtTest: string;
660+
registeredAtTime: number;
667661
fireCount: number;
668662
}
669663
let _reactorIdCounter = 0;
670664
const _reactorMeta = new WeakMap<() => void, ReactorMeta>();
671665
const _classicReactors = new Set<() => void>();
672-
const NO_TEST = '<no-test>';
673666

674667
function _currentTestName(): string {
675668
try {
@@ -679,62 +672,60 @@ function _currentTestName(): string {
679672
} catch {
680673
/* ignore */
681674
}
682-
return NO_TEST;
675+
return '<no-test>';
683676
}
684677

685678
export function registerClassicReactor(cb: () => void, source?: string): () => void {
686679
_classicReactors.add(cb);
687-
const id = ++_reactorIdCounter;
688-
_reactorMeta.set(cb, {
689-
id,
690-
source: source || '<unknown>',
691-
registeredAtTest: _currentTestName(),
692-
fireCount: 0,
693-
});
694-
if ((globalThis as any).__GXT_LEAK_DEBUG__ && (id <= 50 || id % 100 === 0)) {
695-
// eslint-disable-next-line no-console
696-
console.log(
697-
`[leak-debug] registerClassicReactor #${id} source=${source || '?'} test="${_currentTestName()}" total=${_classicReactors.size}`
698-
);
680+
if ((globalThis as any).__GXT_LEAK_DEBUG__) {
681+
const id = ++_reactorIdCounter;
682+
_reactorMeta.set(cb, {
683+
id,
684+
source: source || '<unknown>',
685+
registeredAtTest: _currentTestName(),
686+
registeredAtTime: Date.now(),
687+
fireCount: 0,
688+
});
689+
if (id <= 50 || id % 100 === 0) {
690+
// eslint-disable-next-line no-console
691+
console.log(
692+
`[leak-debug] registerClassicReactor #${id} source=${source || '?'} test="${_currentTestName()}" total=${_classicReactors.size}`
693+
);
694+
}
699695
}
700696
return () => {
701697
_classicReactors.delete(cb);
702-
_reactorMeta.delete(cb);
698+
if ((globalThis as any).__GXT_LEAK_DEBUG__) {
699+
_reactorMeta.delete(cb);
700+
}
703701
};
704702
}
705703

706704
function _fireClassicReactors() {
707705
if (_classicReactors.size === 0) return;
708-
// Copy to avoid mutation during iteration (reactors may unsubscribe
709-
// themselves below, mutating _classicReactors).
706+
// Copy to avoid mutation during iteration
710707
const snapshot = Array.from(_classicReactors);
711708
const debug = (globalThis as any).__GXT_LEAK_DEBUG__;
712-
const currentTest = _currentTestName();
709+
const currentTest = debug ? _currentTestName() : '';
713710
for (const cb of snapshot) {
714-
const meta = _reactorMeta.get(cb);
715-
// Cross-test leak guard: reactors registered during a known test
716-
// that fire during a different known test are leaks. Unsubscribe
717-
// and skip the call. We only enforce this when both registration
718-
// and fire happen with a known test (i.e. neither is <no-test>);
719-
// module-init and pre-QUnit reactors are intentionally global.
720-
if (
721-
meta &&
722-
meta.registeredAtTest !== NO_TEST &&
723-
currentTest !== NO_TEST &&
724-
meta.registeredAtTest !== currentTest
725-
) {
726-
if (debug) {
711+
if (debug) {
712+
const meta = _reactorMeta.get(cb);
713+
if (meta) {
727714
meta.fireCount++;
728-
// eslint-disable-next-line no-console
729-
console.log(
730-
`[leak-debug] DROP reactor #${meta.id} src=${meta.source} regAt="${meta.registeredAtTest}" firedIn="${currentTest}" fires=${meta.fireCount}`
731-
);
715+
// Cross-test leak: reactor registered during a different test
716+
// than the one currently executing.
717+
if (
718+
meta.registeredAtTest !== '<no-test>' &&
719+
currentTest !== '<no-test>' &&
720+
meta.registeredAtTest !== currentTest
721+
) {
722+
// eslint-disable-next-line no-console
723+
console.log(
724+
`[leak-debug] LEAK reactor #${meta.id} src=${meta.source} regAt="${meta.registeredAtTest}" firedIn="${currentTest}" fires=${meta.fireCount}`
725+
);
726+
}
732727
}
733-
_classicReactors.delete(cb);
734-
_reactorMeta.delete(cb);
735-
continue;
736728
}
737-
if (debug && meta) meta.fireCount++;
738729
try {
739730
cb();
740731
} catch {

0 commit comments

Comments
 (0)