Skip to content

Commit 4a9a0a7

Browse files
antonisclaude
andcommitted
fix(core): Evict stale async TurboModule tracker frames
`wrapTurboModule` pops an async frame only when its promise settles. A native method that accepts a promise but never settles it would pin its frame — and, via the scope sync, the native crash scope — for the whole process lifetime, so every later crash is mis-attributed to that call (the Android shape of #6821, but also any custom module passed to `turboModuleContextIntegration({ modules })`). Add an opportunistic, budgeted age sweep on push that drops async frames older than MAX_ASYNC_FRAME_AGE_MS (60s, matching CALLBACK_MAX_AGE_MS), reusing popTurboModuleCall for the scope re-sync. No timers. Bounds the damage to "attribution expires ~60s after the call started" instead of "wrong forever". Sync and callback frames are untouched — the wrapper already pops those synchronously. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a73e0fe commit 4a9a0a7

3 files changed

Lines changed: 162 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212

1313
- Populate the Hermes runtime version on JS profiles instead of sending an empty value ([#6817](https://github.com/getsentry/sentry-react-native/pull/6817))
1414

15+
### Internal
16+
17+
- Evict stale TurboModule tracker frames so a never-settling native promise can't pin crash attribution for the process lifetime
18+
1519
## 8.29.0
1620

1721
### Features

‎packages/core/src/js/turbomodule/turboModuleTracker.ts‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,34 @@ const CONTEXT_KEY = 'turbo_module';
4040
const TAG_NAME = 'turbo_module.name';
4141
const TAG_METHOD = 'turbo_module.method';
4242

43+
/**
44+
* An `async` frame still on the stack after this long is treated as leaked and
45+
* evicted on the next push.
46+
*
47+
* {@link wrapTurboModule} pops an async frame only when its returned promise
48+
* settles. A native method that accepts a promise but never settles it (the
49+
* Android bug in #6821, or a custom module passed to
50+
* `turboModuleContextIntegration({ modules })`) would otherwise pin its frame —
51+
* and, via {@link syncToScope}, the native crash scope — for the entire process
52+
* lifetime, so every later crash is mis-attributed to that call. The sweep
53+
* bounds the damage to "attribution expires ~{@link MAX_ASYNC_FRAME_AGE_MS}
54+
* after the call started" instead of "wrong forever".
55+
*
56+
* Only `async` frames expire: `sync` and callback-style frames are popped
57+
* synchronously by the wrapper (see `wrapTurboModule.ts`), so they can never
58+
* outlive their call. Kept equal to `CALLBACK_MAX_AGE_MS` in
59+
* `turboModuleCallbacks.ts` on purpose — a never-settling call leaks both a
60+
* frame and (if callback-style) a pending-callback entry, and both should age
61+
* out at the same bound.
62+
*/
63+
export const MAX_ASYNC_FRAME_AGE_MS = 60_000;
64+
65+
/**
66+
* How many stale frames a single push may evict. Keeps the sweep amortised O(1)
67+
* on the wrap hot path, mirroring `CALLBACK_SWEEP_BUDGET`.
68+
*/
69+
const ASYNC_FRAME_SWEEP_BUDGET = 8;
70+
4371
let nextCallId = 0;
4472

4573
/**
@@ -98,11 +126,24 @@ export function pushTurboModuleCall(args: {
98126
kind: 'sync' | 'async';
99127
scope?: Scope;
100128
}): number {
129+
const startedAtMs = Date.now();
130+
131+
// Opportunistically drop frames from earlier calls whose promise never
132+
// settled, before this call's attribution is written. `startedAtMs` is taken
133+
// microseconds ago, so reusing it as the cutoff saves a `Date.now()` at the
134+
// cost of an imperceptibly conservative bound (same trick as the callback
135+
// sweep). Isolated so an eviction failure can never block the real push.
136+
try {
137+
evictStaleAsyncFrames(startedAtMs);
138+
} catch {
139+
// ignore — the push below must still happen.
140+
}
141+
101142
const call: InternalCall = {
102143
name: args.name,
103144
method: args.method,
104145
kind: args.kind,
105-
startedAtMs: Date.now(),
146+
startedAtMs,
106147
callId: nextCallId++,
107148
// Default to the isolation scope: it's the one wired up to
108149
// `enableSyncToNative`, so writes here propagate to the native SDKs and
@@ -212,6 +253,38 @@ export function popTurboModuleCall(callId: number): void {
212253
}
213254
}
214255

256+
/**
257+
* Evicts `async` frames older than {@link MAX_ASYNC_FRAME_AGE_MS} — calls whose
258+
* promise never settled, so {@link wrapTurboModule} never popped them. Bounded
259+
* by {@link ASYNC_FRAME_SWEEP_BUDGET} per call to keep the push hot path
260+
* amortised O(1).
261+
*
262+
* Reuses {@link popTurboModuleCall} for each eviction so the scope re-sync /
263+
* clear logic (including the cross-scope native re-sync) lives in exactly one
264+
* place. `callId`s are snapshotted first because `popTurboModuleCall` mutates
265+
* the stack. Only `async` frames are considered: `sync` and callback-style
266+
* frames are always popped synchronously by the wrapper.
267+
*/
268+
function evictStaleAsyncFrames(nowMs: number): void {
269+
let staleIds: number[] | undefined;
270+
let budget = ASYNC_FRAME_SWEEP_BUDGET;
271+
for (const frame of stack) {
272+
if (budget <= 0) {
273+
break;
274+
}
275+
if (frame.kind === 'async' && nowMs - frame.startedAtMs > MAX_ASYNC_FRAME_AGE_MS) {
276+
(staleIds ??= []).push(frame.callId);
277+
budget--;
278+
}
279+
}
280+
if (!staleIds) {
281+
return;
282+
}
283+
for (const callId of staleIds) {
284+
popTurboModuleCall(callId);
285+
}
286+
}
287+
215288
function syncToScope(call: InternalCall): void {
216289
call.scope.setContext(CONTEXT_KEY, {
217290
name: call.name,

‎packages/core/test/turbomodule/turboModuleTracker.test.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
_resetTurboModuleTracker,
55
getActiveTurboModuleCall,
66
getTurboModuleCallStack,
7+
MAX_ASYNC_FRAME_AGE_MS,
78
popTurboModuleCall,
89
pushTurboModuleCall,
910
relabelTurboModuleCallKind,
@@ -264,4 +265,87 @@ describe('turboModuleTracker', () => {
264265
expect(b).toBe(a + 1);
265266
expect(c).toBe(b + 1);
266267
});
268+
269+
describe('stale async frame eviction', () => {
270+
beforeEach(() => {
271+
jest.useFakeTimers();
272+
jest.setSystemTime(0);
273+
});
274+
275+
afterEach(() => {
276+
jest.useRealTimers();
277+
});
278+
279+
it('evicts an async frame whose promise never settled on the next push', () => {
280+
// Simulates the #6821 shape: an async call pushed at startup that the
281+
// wrapper never pops because the native promise never settles.
282+
pushTurboModuleCall({
283+
name: 'RNSentry',
284+
method: 'initNativeReactNavigationNewFrameTracking',
285+
kind: 'async',
286+
scope,
287+
});
288+
expect(getTurboModuleCallStack()).toHaveLength(1);
289+
290+
jest.setSystemTime(MAX_ASYNC_FRAME_AGE_MS + 1);
291+
const freshId = pushTurboModuleCall({ name: 'RNSentry', method: 'captureEnvelope', kind: 'async', scope });
292+
293+
// The leaked frame is gone; only the fresh call remains.
294+
const remaining = getTurboModuleCallStack();
295+
expect(remaining).toHaveLength(1);
296+
expect(remaining[0]).toMatchObject({ method: 'captureEnvelope', callId: freshId });
297+
298+
// The scope now attributes the fresh call, not the leaked one.
299+
expect(scope.getScopeData().contexts.turbo_module).toMatchObject({ method: 'captureEnvelope' });
300+
});
301+
302+
it('fully clears the scope once a leaked frame is evicted and its replacement pops', () => {
303+
pushTurboModuleCall({ name: 'RNSentry', method: 'leaky', kind: 'async', scope });
304+
305+
jest.setSystemTime(MAX_ASYNC_FRAME_AGE_MS + 1);
306+
const freshId = pushTurboModuleCall({ name: 'RNSentry', method: 'captureEnvelope', kind: 'async', scope });
307+
popTurboModuleCall(freshId);
308+
309+
// If the leaked frame were still lurking, the scope would be pinned to it.
310+
expect(getTurboModuleCallStack()).toEqual([]);
311+
expect(scope.getScopeData().contexts.turbo_module).toBeUndefined();
312+
expect(scope.getScopeData().tags['turbo_module.method']).toBe('');
313+
});
314+
315+
it('does not evict a frame that is within the age bound', () => {
316+
pushTurboModuleCall({ name: 'RNSentry', method: 'inFlight', kind: 'async', scope });
317+
318+
jest.setSystemTime(MAX_ASYNC_FRAME_AGE_MS - 1);
319+
pushTurboModuleCall({ name: 'RNSentry', method: 'captureEnvelope', kind: 'async', scope });
320+
321+
expect(getTurboModuleCallStack()).toHaveLength(2);
322+
});
323+
324+
it('only evicts async frames, never sync frames', () => {
325+
// Sync frames are always popped synchronously by the wrapper, so the age
326+
// sweep must leave them alone even if one is somehow older than the bound.
327+
pushTurboModuleCall({ name: 'RNSentry', method: 'syncButOld', kind: 'sync', scope });
328+
329+
jest.setSystemTime(MAX_ASYNC_FRAME_AGE_MS + 1);
330+
pushTurboModuleCall({ name: 'RNSentry', method: 'captureEnvelope', kind: 'async', scope });
331+
332+
const methods = getTurboModuleCallStack().map(c => c.method);
333+
expect(methods).toEqual(['syncButOld', 'captureEnvelope']);
334+
});
335+
336+
it('evicts multiple leaked frames in one push, up to the sweep budget', () => {
337+
// 10 leaked async frames, sweep budget is 8.
338+
for (let i = 0; i < 10; i++) {
339+
pushTurboModuleCall({ name: 'RNSentry', method: `leak${i}`, kind: 'async', scope });
340+
}
341+
expect(getTurboModuleCallStack()).toHaveLength(10);
342+
343+
jest.setSystemTime(MAX_ASYNC_FRAME_AGE_MS + 1);
344+
pushTurboModuleCall({ name: 'RNSentry', method: 'fresh', kind: 'async', scope });
345+
346+
// 10 leaked - 8 swept + 1 fresh = 3 remaining. The budget keeps the sweep
347+
// amortised O(1); the leftover frames age out on subsequent pushes.
348+
expect(getTurboModuleCallStack()).toHaveLength(3);
349+
});
350+
});
267351
});

0 commit comments

Comments
 (0)