Skip to content

Commit 34f9515

Browse files
lifeartclaude
andcommitted
debug(gxt): instrument cumulative-state leak path
Adds opt-in diagnostic logging behind globalThis.__GXT_LEAK_DEBUG__ (force-enabled in index.html for this CI run; revert once diagnosed). Instrumentation: - validator.ts: tags every classic reactor with {id, source, registered- AtTest} via WeakMap. _fireClassicReactors logs cross-test fires as "LEAK reactor #N src=… regAt=… firedIn=…". __gxtLeakSnapshot dumps the live reactor population grouped by source/origin-test. - renderer.ts: tags _renderComponentGxt's reactor with source= "_renderComponentGxt". - manager.ts: tags renderLinkToElement's reactor with source= "renderLinkToElement"; logs every cache-HIT __syncFromUpstream call with the live/upstream textarea values — the actual clobber site identified for the Textarea cluster. - index.html: snapshots the reactor Set at testStart / testDone so growth between adjacent tests is visible. Also adds scripts/gxt-test-runner/leak-debug.mjs — a Playwright helper that opens the dev server, captures browser console.log output, and prints "[leak-debug]" lines so the multi-module cumulative-state run can be reproduced locally without testem. This is a debugging commit, not a fix. Prior 14+ attempts treated the classic-reactor leak as the root cause but neither global drains nor self-unsub via isConnected/marker tracking improved the failing 8 testem tests; the actual leak source still needs to be identified from CI evidence. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 6ee5180 commit 34f9515

5 files changed

Lines changed: 206 additions & 5 deletions

File tree

index.html

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@
6060
// after each test, preventing phantom failures from leaked state.
6161

6262
QUnit.testStart(function () {
63+
// Leak-debug: record reactor population at start of each test
64+
// so the cross-test leak signal becomes visible against a known
65+
// baseline. The label includes the upcoming test's name.
66+
try {
67+
if (typeof globalThis.__gxtLeakSnapshot === 'function') {
68+
globalThis.__gxtLeakSnapshot('testStart:' + (QUnit.config.current?.testName || ''));
69+
}
70+
} catch (_e) {
71+
/* ignore */
72+
}
6373
// Signal that we're no longer in a test transition
6474
globalThis.__gxtTestTransition = false;
6575
// Reset the operation counter so each test gets a fresh budget
@@ -91,6 +101,15 @@
91101
});
92102

93103
QUnit.testDone(function () {
104+
// Leak-debug: record reactor population at end of each test.
105+
// A reactor that survives the cleanup hook is a confirmed leak.
106+
try {
107+
if (typeof globalThis.__gxtLeakSnapshot === 'function') {
108+
globalThis.__gxtLeakSnapshot('testDone:' + (QUnit.config.current?.testName || ''));
109+
}
110+
} catch (_e) {
111+
/* ignore */
112+
}
94113
// Signal that we're between tests — interval timer should not fire
95114
globalThis.__gxtTestTransition = true;
96115
// Safety net: reset syncing flag after each test in case it got stuck
@@ -111,6 +130,22 @@
111130
// during module initialization (before compile.ts sets it)
112131
globalThis.__GXT_MODE__ = true;
113132

133+
// Leak diagnostics: enable via QUnit URL param (?gxtLeakDebug=true)
134+
// or by setting globalThis.__GXT_LEAK_DEBUG__ before tests start.
135+
// When on, validator.ts tags every classic reactor with the test
136+
// it was registered in and logs cross-test leaks at fire time.
137+
try {
138+
var _u = new URL(location.href);
139+
if (_u.searchParams.get('gxtLeakDebug') === 'true') {
140+
globalThis.__GXT_LEAK_DEBUG__ = true;
141+
}
142+
} catch (_e) {
143+
/* ignore */
144+
}
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;
148+
114149
// Global operation counter to detect infinite synchronous loops
115150
// Any tight loop will increment this. When it exceeds the limit,
116151
// we throw to break out of the loop.

packages/@ember/-internals/glimmer/lib/renderer.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,10 @@ const touchClassicBridge: () => void =
114114
typeof (_glimmerValidator as any).touchClassicBridge === 'function'
115115
? (_glimmerValidator as any).touchClassicBridge
116116
: () => {};
117-
const registerClassicReactor: (cb: () => void) => () => void =
117+
const registerClassicReactor: (cb: () => void, source?: string) => () => void =
118118
typeof (_glimmerValidator as any).registerClassicReactor === 'function'
119119
? (_glimmerValidator as any).registerClassicReactor
120-
: (_: () => void) => () => {};
120+
: (_: () => void, _src?: string) => () => {};
121121
import { tagForObject } from '@ember/-internals/metal';
122122
import type { SimpleDocument, SimpleElement, SimpleNode } from '@simple-dom/interface';
123123
import RSVP from 'rsvp';
@@ -2148,7 +2148,7 @@ function _renderComponentGxt(
21482148
// detached-element use case).
21492149
}
21502150
fireReactor();
2151-
});
2151+
}, '_renderComponentGxt');
21522152
}
21532153

21542154
try {

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9516,7 +9516,7 @@ function renderLinkToElement(instance: any, args: any, fw: any): HTMLAnchorEleme
95169516
}
95179517
}
95189518
};
9519-
unsub = _gxtRegisterClassicReactor(wrapped);
9519+
unsub = (_gxtRegisterClassicReactor as any)(wrapped, 'renderLinkToElement');
95209520
};
95219521

95229522
// id attribute (doesn't change over the element's lifetime, so no reactor)
@@ -10067,6 +10067,26 @@ function handleManagedComponent(
1006710067
// primitive value equals (a new LocalValue may have been assigned
1006810068
// and the old effect is disconnected from it).
1006910069
if (inst && inst._value && typeof inst._value.__syncFromUpstream === 'function') {
10070+
// Leak-debug: this is the final clobber site for typed input
10071+
// values. A cache HIT here on the next test (after a leaked
10072+
// reactor triggered re-render) wipes user input.
10073+
if ((globalThis as any).__GXT_LEAK_DEBUG__) {
10074+
try {
10075+
const Q = (globalThis as any).QUnit;
10076+
const t = Q?.config?.current;
10077+
const liveVal =
10078+
cached.liveEl && cached.liveEl.isConnected
10079+
? (cached.liveEl as HTMLInputElement).value
10080+
: '<no-live>';
10081+
const upstreamVal = inst.value;
10082+
// eslint-disable-next-line no-console
10083+
console.log(
10084+
`[leak-debug] cacheHIT __syncFromUpstream slot=${slotKey} test="${t?.module?.name || ''}::${t?.testName || ''}" live=${JSON.stringify(liveVal)} upstream=${JSON.stringify(upstreamVal)}`
10085+
);
10086+
} catch {
10087+
/* ignore */
10088+
}
10089+
}
1007010090
inst._value.__syncFromUpstream();
1007110091
if (cached.liveEl && cached.liveEl.isConnected) {
1007210092
try {

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

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,18 +648,84 @@ 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+
// === 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).
656+
interface ReactorMeta {
657+
id: number;
658+
source: string;
659+
registeredAtTest: string;
660+
registeredAtTime: number;
661+
fireCount: number;
662+
}
663+
let _reactorIdCounter = 0;
664+
const _reactorMeta = new WeakMap<() => void, ReactorMeta>();
651665
const _classicReactors = new Set<() => void>();
652-
export function registerClassicReactor(cb: () => void): () => void {
666+
667+
function _currentTestName(): string {
668+
try {
669+
const Q = (globalThis as any).QUnit;
670+
const t = Q?.config?.current;
671+
if (t) return `${t.module?.name || ''} :: ${t.testName || ''}`;
672+
} catch {
673+
/* ignore */
674+
}
675+
return '<no-test>';
676+
}
677+
678+
export function registerClassicReactor(cb: () => void, source?: string): () => void {
653679
_classicReactors.add(cb);
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+
}
695+
}
654696
return () => {
655697
_classicReactors.delete(cb);
698+
if ((globalThis as any).__GXT_LEAK_DEBUG__) {
699+
_reactorMeta.delete(cb);
700+
}
656701
};
657702
}
703+
658704
function _fireClassicReactors() {
659705
if (_classicReactors.size === 0) return;
660706
// Copy to avoid mutation during iteration
661707
const snapshot = Array.from(_classicReactors);
708+
const debug = (globalThis as any).__GXT_LEAK_DEBUG__;
709+
const currentTest = debug ? _currentTestName() : '';
662710
for (const cb of snapshot) {
711+
if (debug) {
712+
const meta = _reactorMeta.get(cb);
713+
if (meta) {
714+
meta.fireCount++;
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+
}
727+
}
728+
}
663729
try {
664730
cb();
665731
} catch {
@@ -668,6 +734,25 @@ function _fireClassicReactors() {
668734
}
669735
}
670736

737+
// Snapshot the reactor population at a checkpoint (testStart / testDone).
738+
(globalThis as any).__gxtLeakSnapshot = function (label: string) {
739+
if (!(globalThis as any).__GXT_LEAK_DEBUG__) return;
740+
const buckets = new Map<string, number>();
741+
for (const cb of _classicReactors) {
742+
const meta = _reactorMeta.get(cb);
743+
const key = meta
744+
? `src=${meta.source} regAt="${meta.registeredAtTest}"`
745+
: '<no-meta>';
746+
buckets.set(key, (buckets.get(key) || 0) + 1);
747+
}
748+
// eslint-disable-next-line no-console
749+
console.log(`[leak-debug] === ${label} === total reactors=${_classicReactors.size}`);
750+
for (const [key, count] of buckets) {
751+
// eslint-disable-next-line no-console
752+
console.log(`[leak-debug] ${count}× ${key}`);
753+
}
754+
};
755+
671756
// Wrap dirtyTagFor to also bump the global revision AND mark the specific tag as dirty
672757
const gxtDirtyTagFor = validator.dirtyTagFor;
673758

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Open the local vite dev server in a single browser context, let QUnit run
2+
// all tests naturally, and capture browser console output to stdout. The
3+
// __GXT_LEAK_DEBUG__ flag in index.html turns on the instrumentation in
4+
// validator.ts/manager.ts/renderer.ts.
5+
//
6+
// Usage:
7+
// node scripts/gxt-test-runner/leak-debug.mjs > /tmp/leak.log
8+
// grep -E 'LEAK|cacheHIT' /tmp/leak.log
9+
//
10+
// Tunable: GXT_LEAK_TIMEOUT_MS (default 1800000 = 30min).
11+
12+
import { chromium } from 'playwright';
13+
14+
const URL = process.env.GXT_URL || 'http://localhost:5180/';
15+
const TIMEOUT = Number(process.env.GXT_LEAK_TIMEOUT_MS || 1_800_000);
16+
17+
const browser = await chromium.launch({ headless: true });
18+
const ctx = await browser.newContext();
19+
const page = await ctx.newPage();
20+
21+
let lines = 0;
22+
let leaks = 0;
23+
let cacheHits = 0;
24+
page.on('console', (msg) => {
25+
const text = msg.text();
26+
if (
27+
text.includes('[leak-debug]') ||
28+
text.includes(' LEAK ') ||
29+
text.includes('cacheHIT')
30+
) {
31+
if (text.includes('LEAK')) leaks++;
32+
if (text.includes('cacheHIT')) cacheHits++;
33+
lines++;
34+
process.stdout.write(text + '\n');
35+
}
36+
});
37+
38+
page.on('pageerror', (err) => {
39+
process.stderr.write('[pageerror] ' + err.message + '\n');
40+
});
41+
42+
await page.goto(URL, { waitUntil: 'load', timeout: 60_000 });
43+
44+
// Wait for QUnit to complete OR our wall-clock timeout.
45+
const result = await page.evaluate((timeoutMs) => {
46+
return new Promise((resolve) => {
47+
if (typeof QUnit === 'undefined') {
48+
resolve({ status: 'no-qunit' });
49+
return;
50+
}
51+
const t = setTimeout(() => resolve({ status: 'timeout' }), timeoutMs);
52+
QUnit.done((rec) => {
53+
clearTimeout(t);
54+
resolve({ status: 'done', failed: rec.failed, passed: rec.passed, total: rec.total });
55+
});
56+
});
57+
}, TIMEOUT);
58+
59+
process.stdout.write(`---summary--- ${JSON.stringify(result)} lines=${lines} leaks=${leaks} cacheHits=${cacheHits}\n`);
60+
61+
await browser.close();

0 commit comments

Comments
 (0)