From e0af87f997167126f483e748620621d35003029e Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Wed, 20 May 2026 09:05:44 +0200 Subject: [PATCH 1/4] fix(core): Use WeakRef for Span-Scope circular references Break the circular reference between Span and Scope by using WeakRef in both directions, allowing proper garbage collection of completed spans and their associated scopes. Co-Authored-By: Claude Opus 4.5 --- packages/core/src/tracing/utils.ts | 8 ++- packages/core/src/utils/spanOnScope.ts | 8 +-- packages/core/src/utils/spanUtils.ts | 34 +++++++++---- packages/core/test/lib/tracing/trace.test.ts | 7 +-- packages/core/test/lib/tracing/utils.test.ts | 49 +++++++++---------- packages/opentelemetry/src/spanExporter.ts | 31 ++++-------- .../test/integration/transactions.test.ts | 25 +++------- 7 files changed, 74 insertions(+), 88 deletions(-) diff --git a/packages/core/src/tracing/utils.ts b/packages/core/src/tracing/utils.ts index 0c0417a71a90..e9c8b830af55 100644 --- a/packages/core/src/tracing/utils.ts +++ b/packages/core/src/tracing/utils.ts @@ -7,7 +7,7 @@ const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope'; type SpanWithScopes = Span & { - [SCOPE_ON_START_SPAN_FIELD]?: Scope; + [SCOPE_ON_START_SPAN_FIELD]?: MaybeWeakRef; [ISOLATION_SCOPE_ON_START_SPAN_FIELD]?: MaybeWeakRef; }; @@ -15,9 +15,7 @@ type SpanWithScopes = Span & { export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, isolationScope: Scope): void { if (span) { addNonEnumerableProperty(span, ISOLATION_SCOPE_ON_START_SPAN_FIELD, makeWeakRef(isolationScope)); - // We don't wrap the scope with a WeakRef here because webkit aggressively garbage collects - // and scopes are not held in memory for long periods of time. - addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, scope); + addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, makeWeakRef(scope)); } } @@ -29,7 +27,7 @@ export function getCapturedScopesOnSpan(span: Span): { scope?: Scope; isolationS const spanWithScopes = span as SpanWithScopes; return { - scope: spanWithScopes[SCOPE_ON_START_SPAN_FIELD], + scope: derefWeakRef(spanWithScopes[SCOPE_ON_START_SPAN_FIELD]), isolationScope: derefWeakRef(spanWithScopes[ISOLATION_SCOPE_ON_START_SPAN_FIELD]), }; } diff --git a/packages/core/src/utils/spanOnScope.ts b/packages/core/src/utils/spanOnScope.ts index 0e95d63984fa..ff1e5983895d 100644 --- a/packages/core/src/utils/spanOnScope.ts +++ b/packages/core/src/utils/spanOnScope.ts @@ -1,11 +1,12 @@ import type { Scope } from '../scope'; import type { Span } from '../types/span'; import { addNonEnumerableProperty } from '../utils/object'; +import { derefWeakRef, makeWeakRef, type MaybeWeakRef } from './weakRef'; const SCOPE_SPAN_FIELD = '_sentrySpan'; type ScopeWithMaybeSpan = Scope & { - [SCOPE_SPAN_FIELD]?: Span; + [SCOPE_SPAN_FIELD]?: MaybeWeakRef; }; /** @@ -14,7 +15,8 @@ type ScopeWithMaybeSpan = Scope & { */ export function _setSpanForScope(scope: Scope, span: Span | undefined): void { if (span) { - addNonEnumerableProperty(scope as ScopeWithMaybeSpan, SCOPE_SPAN_FIELD, span); + // Use WeakRef to avoid circular reference with span holding scope + addNonEnumerableProperty(scope as ScopeWithMaybeSpan, SCOPE_SPAN_FIELD, makeWeakRef(span)); } else { // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete (scope as ScopeWithMaybeSpan)[SCOPE_SPAN_FIELD]; @@ -26,5 +28,5 @@ export function _setSpanForScope(scope: Scope, span: Span | undefined): void { * NOTE: This should NOT be used directly, but is only used internally by the trace methods. */ export function _getSpanForScope(scope: ScopeWithMaybeSpan): Span | undefined { - return scope[SCOPE_SPAN_FIELD]; + return derefWeakRef(scope[SCOPE_SPAN_FIELD]); } diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 9f495ef7b30e..039031db963b 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -1,3 +1,5 @@ +/* eslint-disable max-lines */ + import { getAsyncContextStrategy } from '../asyncContext'; import type { RawAttributes } from '../attributes'; import { serializeAttributes } from '../attributes'; @@ -30,6 +32,8 @@ import { timestampInSeconds } from '../utils/time'; import { generateSentryTraceHeader, generateTraceparentHeader } from '../utils/tracing'; import { consoleSandbox } from './debug-logger'; import { _getSpanForScope } from './spanOnScope'; +import type { MaybeWeakRef } from './weakRef'; +import { derefWeakRef, makeWeakRef } from './weakRef'; // These are aligned with OpenTelemetry trace flags export const TRACE_FLAG_NONE = 0x0; @@ -343,12 +347,13 @@ const CHILD_SPANS_FIELD = '_sentryChildSpans'; const ROOT_SPAN_FIELD = '_sentryRootSpan'; type SpanWithPotentialChildren = Span & { - [CHILD_SPANS_FIELD]?: Set; + [CHILD_SPANS_FIELD]?: Set>; [ROOT_SPAN_FIELD]?: Span; }; /** * Adds an opaque child span reference to a span. + * Uses WeakRef to allow child spans to be garbage collected when no longer needed. */ export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void { // We store the root span reference on the child span @@ -356,19 +361,25 @@ export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: S const rootSpan = span[ROOT_SPAN_FIELD] || span; addNonEnumerableProperty(childSpan as SpanWithPotentialChildren, ROOT_SPAN_FIELD, rootSpan); - // We store a list of child spans on the parent span - // We need this for `getSpanDescendants()` to work + const ref = makeWeakRef(childSpan); + if (span[CHILD_SPANS_FIELD]) { - span[CHILD_SPANS_FIELD].add(childSpan); + span[CHILD_SPANS_FIELD].add(ref); } else { - addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([childSpan])); + addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([ref])); } } /** This is only used internally by Idle Spans. */ export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void { - if (span[CHILD_SPANS_FIELD]) { - span[CHILD_SPANS_FIELD].delete(childSpan); + const children = span[CHILD_SPANS_FIELD]; + if (children) { + for (const ref of children) { + if (derefWeakRef(ref) === childSpan) { + children.delete(ref); + break; + } + } } } @@ -385,9 +396,12 @@ export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] { // We want to ignore unsampled spans (e.g. non recording spans) } else if (spanIsSampled(span)) { resultSet.add(span); - const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : []; - for (const childSpan of childSpans) { - addSpanChildren(childSpan); + const childRefs = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : []; + for (const ref of childRefs) { + const childSpan = derefWeakRef(ref); + if (childSpan) { + addSpanChildren(childSpan); + } } } } diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 0d0e652e64aa..451ed38c706f 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -13,6 +13,7 @@ import { spanToJSON, withScope, } from '../../../src'; +import { derefWeakRef } from '../../../src/utils/weakRef'; import { getAsyncContextStrategy } from '../../../src/asyncContext'; import { continueTrace, @@ -786,7 +787,7 @@ describe('startSpan', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { startSpan({ name: 'inner' }, innerSpan => { - const childSpans = Array.from(outerSpan._sentryChildSpans); + const childSpans = Array.from(outerSpan._sentryChildSpans).map(derefWeakRef); expect(childSpans).toContain(innerSpan); }); }); @@ -1335,7 +1336,7 @@ describe('startSpanManual', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { startSpanManual({ name: 'inner' }, innerSpan => { - const childSpans = Array.from(outerSpan._sentryChildSpans); + const childSpans = Array.from(outerSpan._sentryChildSpans).map(derefWeakRef); expect(childSpans).toContain(innerSpan); }); }); @@ -1802,7 +1803,7 @@ describe('startInactiveSpan', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { const innerSpan = startInactiveSpan({ name: 'inner' }); - const childSpans = Array.from(outerSpan._sentryChildSpans); + const childSpans = Array.from(outerSpan._sentryChildSpans).map(derefWeakRef); expect(childSpans).toContain(innerSpan); }); }); diff --git a/packages/core/test/lib/tracing/utils.test.ts b/packages/core/test/lib/tracing/utils.test.ts index 3f65bd89ef9a..6871f13a4698 100644 --- a/packages/core/test/lib/tracing/utils.test.ts +++ b/packages/core/test/lib/tracing/utils.test.ts @@ -44,17 +44,17 @@ describe('tracing utils', () => { expect(retrieved.isolationScope).toBeUndefined(); }); - it('uses WeakRef only for isolation scopes', () => { + it('uses WeakRef for both scopes', () => { const span = createMockSpan(); const scope = new Scope(); const isolationScope = new Scope(); setCapturedScopesOnSpan(span, scope, isolationScope); - // Check that only isolation scope is wrapped with WeakRef + // Check that both scopes are wrapped with WeakRef const spanWithScopes = span as any; - expect(spanWithScopes._sentryScope).toBe(scope); // Regular scope stored directly - expect(spanWithScopes._sentryIsolationScope).toBeInstanceOf(WeakRef); // Isolation scope wrapped + expect(spanWithScopes._sentryScope).toBeInstanceOf(WeakRef); + expect(spanWithScopes._sentryIsolationScope).toBeInstanceOf(WeakRef); // Verify we can still retrieve the scopes const retrieved = getCapturedScopesOnSpan(span); @@ -77,14 +77,8 @@ describe('tracing utils', () => { // Check that both scopes are stored directly when WeakRef is not available const spanWithScopes = span as any; - expect(spanWithScopes._sentryScope).toBe(scope); // Regular scope always stored directly - expect(spanWithScopes._sentryIsolationScope).toBe(isolationScope); // Isolation scope falls back to direct storage - - // When WeakRef is available, ensure regular scope is not wrapped but isolation scope would be - if (originalWeakRef) { - expect(spanWithScopes._sentryScope).not.toBeInstanceOf(originalWeakRef); - expect(spanWithScopes._sentryIsolationScope).not.toBeInstanceOf(originalWeakRef); - } + expect(spanWithScopes._sentryScope).toBe(scope); + expect(spanWithScopes._sentryIsolationScope).toBe(isolationScope); // Verify we can still retrieve the scopes const retrieved = getCapturedScopesOnSpan(span); @@ -98,35 +92,36 @@ describe('tracing utils', () => { it('handles WeakRef deref returning undefined gracefully', () => { const span = createMockSpan(); - const scope = new Scope(); - const isolationScope = new Scope(); - - setCapturedScopesOnSpan(span, scope, isolationScope); - // Mock WeakRef.deref to return undefined for isolation scope (simulating garbage collection) - // Regular scope is stored directly, so it should always be available + // Mock WeakRef.deref to return undefined (simulating garbage collection) const spanWithScopes = span as any; + const mockScopeWeakRef = { + deref: vi.fn().mockReturnValue(undefined), + }; const mockIsolationScopeWeakRef = { deref: vi.fn().mockReturnValue(undefined), }; - // Keep the regular scope as is (stored directly) - // Only replace the isolation scope with a mock WeakRef + spanWithScopes._sentryScope = mockScopeWeakRef; spanWithScopes._sentryIsolationScope = mockIsolationScopeWeakRef; const retrieved = getCapturedScopesOnSpan(span); - expect(retrieved.scope).toBe(scope); // Regular scope should still be available - expect(retrieved.isolationScope).toBeUndefined(); // Isolation scope should be undefined due to GC + expect(retrieved.scope).toBeUndefined(); + expect(retrieved.isolationScope).toBeUndefined(); + expect(mockScopeWeakRef.deref).toHaveBeenCalled(); expect(mockIsolationScopeWeakRef.deref).toHaveBeenCalled(); }); it('handles corrupted WeakRef objects gracefully', () => { const span = createMockSpan(); - const scope = new Scope(); - // Set up a regular scope (stored directly) and a corrupted isolation scope WeakRef + // Set up corrupted WeakRefs that throw on deref const spanWithScopes = span as any; - spanWithScopes._sentryScope = scope; // Regular scope stored directly + spanWithScopes._sentryScope = { + deref: vi.fn().mockImplementation(() => { + throw new Error('WeakRef deref failed'); + }), + }; spanWithScopes._sentryIsolationScope = { deref: vi.fn().mockImplementation(() => { throw new Error('WeakRef deref failed'); @@ -134,8 +129,8 @@ describe('tracing utils', () => { }; const retrieved = getCapturedScopesOnSpan(span); - expect(retrieved.scope).toBe(scope); // Regular scope should still be available - expect(retrieved.isolationScope).toBeUndefined(); // Isolation scope should be undefined due to error + expect(retrieved.scope).toBeUndefined(); + expect(retrieved.isolationScope).toBeUndefined(); }); it('preserves scope data when using WeakRef', () => { diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index aed57b52e58e..b050c92a583d 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -20,6 +20,7 @@ import { getCapturedScopesOnSpan, getDynamicSamplingContextFromSpan, getStatusMessage, + LRUMap, SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -41,6 +42,7 @@ type SpanNodeCompleted = SpanNode & { span: ReadableSpan }; const MAX_SPAN_COUNT = 1000; const DEFAULT_TIMEOUT = 300; // 5 min +const SENT_SPANS_MAX_SIZE = 10000; interface FinishedSpanBucket { timestampInS: number; @@ -71,9 +73,7 @@ export class SentrySpanExporter { private _finishedSpanBucketSize: number; private _spansToBucketEntry: WeakMap; private _lastCleanupTimestampInS: number; - // Essentially a a set of span ids that are already sent. The values are expiration - // times in this cache so we don't hold onto them indefinitely. - private _sentSpans: Map; + private _sentSpans: LRUMap; /* Internally, we use a debounced flush to give some wiggle room to the span processor to accumulate more spans. */ private _debouncedFlush: ReturnType; @@ -85,7 +85,7 @@ export class SentrySpanExporter { this._finishedSpanBuckets = new Array(this._finishedSpanBucketSize).fill(undefined); this._lastCleanupTimestampInS = Math.floor(_INTERNAL_safeDateNow() / 1000); this._spansToBucketEntry = new WeakMap(); - this._sentSpans = new Map(); + this._sentSpans = new LRUMap(SENT_SPANS_MAX_SIZE); this._debouncedFlush = debounce(this.flush.bind(this), 1, { maxWait: 100 }); } @@ -124,7 +124,7 @@ export class SentrySpanExporter { // If the span doesn't have a local parent ID (it's a root span), we're gonna flush all the ended spans const localParentId = getLocalParentId(span); - if (!localParentId || this._sentSpans.has(localParentId)) { + if (!localParentId || this._sentSpans.get(localParentId)) { this._debouncedFlush(); } } @@ -137,7 +137,6 @@ export class SentrySpanExporter { public flush(): void { const finishedSpans = this._finishedSpanBuckets.flatMap(bucket => (bucket ? Array.from(bucket.spans) : [])); - this._flushSentSpanCache(); const sentSpans = this._maybeSend(finishedSpans); const sentSpanCount = sentSpans.size; @@ -147,15 +146,14 @@ export class SentrySpanExporter { `SpanExporter exported ${sentSpanCount} spans, ${remainingOpenSpanCount} spans are waiting for their parent spans to finish`, ); - const expirationDate = _INTERNAL_safeDateNow() + DEFAULT_TIMEOUT * 1000; - for (const span of sentSpans) { - this._sentSpans.set(span.spanContext().spanId, expirationDate); + this._sentSpans.set(span.spanContext().spanId, 1); const bucketEntry = this._spansToBucketEntry.get(span); if (bucketEntry) { bucketEntry.spans.delete(span); } } + // Cancel a pending debounced flush, if there is one // This can be relevant if we directly flush, circumventing the debounce // in that case, we want to cancel any pending debounced flush @@ -193,7 +191,7 @@ export class SentrySpanExporter { const transactionEvent = createTransactionForOtelSpan(span); // Add an attribute to the transaction event to indicate that this transaction is an orphaned transaction - if (root.parentNode && this._sentSpans.has(root.parentNode.id)) { + if (root.parentNode && this._sentSpans.get(root.parentNode.id)) { const traceData = transactionEvent.contexts?.trace?.data; if (traceData) { traceData['sentry.parent_span_already_sent'] = true; @@ -235,20 +233,9 @@ export class SentrySpanExporter { return sentSpans; } - /** Remove "expired" span id entries from the _sentSpans cache. */ - private _flushSentSpanCache(): void { - const currentTimestamp = _INTERNAL_safeDateNow(); - // Note, it is safe to delete items from the map as we go: https://stackoverflow.com/a/35943995/90297 - for (const [spanId, expirationTime] of this._sentSpans.entries()) { - if (expirationTime <= currentTimestamp) { - this._sentSpans.delete(spanId); - } - } - } - /** Check if a node is a completed root node or a node whose parent has already been sent */ private _nodeIsCompletedRootNodeOrHasSentParent(node: SpanNode): node is SpanNodeCompleted { - return !!node.span && (!node.parentNode || this._sentSpans.has(node.parentNode.id)); + return !!node.span && (!node.parentNode || !!this._sentSpans.get(node.parentNode.id)); } /** Get all completed root nodes from a list of nodes */ diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index aae975d8d8a9..7280bc4d9b0c 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -612,7 +612,7 @@ describe('Integration | Transactions', () => { expect(finishedSpans.length).toBe(0); }); - it('discards child spans that are finished after 5 minutes their parent span has been sent', async () => { + it('sends child spans that are finished after 5 minutes as orphaned transactions', async () => { const timeout = 5 * 60 * 1000; const now = Date.now(); vi.useFakeTimers(); @@ -631,14 +631,6 @@ describe('Integration | Transactions', () => { }, }); - const spanProcessor = getSpanProcessor(); - - const exporter = spanProcessor ? spanProcessor['_exporter'] : undefined; - - if (!exporter) { - throw new Error('No exporter found, aborting test...'); - } - startSpanManual({ name: 'test name' }, async span => { const subSpan = startInactiveSpan({ name: 'inner span 1' }); subSpan.end(); @@ -654,18 +646,15 @@ describe('Integration | Transactions', () => { vi.advanceTimersByTime(timeout + 2); - expect(transactions).toHaveLength(1); + expect(transactions).toHaveLength(2); expect(transactions[0]?.spans).toHaveLength(1); - // subSpan2 is pending (and will eventually be cleaned up) - const finishedSpans: any = []; - exporter['_finishedSpanBuckets'].forEach(bucket => { - if (bucket) { - finishedSpans.push(...bucket.spans); - } + expect(transactions[1]?.transaction).toBe('inner span 2'); + expect(transactions[1]?.contexts?.trace?.data).toEqual({ + 'sentry.parent_span_already_sent': true, + 'sentry.origin': 'manual', + 'sentry.source': 'custom', }); - expect(finishedSpans.length).toBe(1); - expect(finishedSpans[0]?.name).toBe('inner span 2'); }); it('only considers sent spans, not finished spans, for flushing orphaned spans of sent spans', async () => { From 8cc1d71464bb3f638fffe5c4e2e2e921516dcd35 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 29 May 2026 13:17:44 +0200 Subject: [PATCH 2/4] fixup! fix(core): Use WeakRef for Span-Scope circular references --- packages/core/src/tracing/utils.ts | 8 +++-- packages/core/test/lib/tracing/utils.test.ts | 32 ++++++++------------ 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/packages/core/src/tracing/utils.ts b/packages/core/src/tracing/utils.ts index e9c8b830af55..0c0417a71a90 100644 --- a/packages/core/src/tracing/utils.ts +++ b/packages/core/src/tracing/utils.ts @@ -7,7 +7,7 @@ const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope'; type SpanWithScopes = Span & { - [SCOPE_ON_START_SPAN_FIELD]?: MaybeWeakRef; + [SCOPE_ON_START_SPAN_FIELD]?: Scope; [ISOLATION_SCOPE_ON_START_SPAN_FIELD]?: MaybeWeakRef; }; @@ -15,7 +15,9 @@ type SpanWithScopes = Span & { export function setCapturedScopesOnSpan(span: Span | undefined, scope: Scope, isolationScope: Scope): void { if (span) { addNonEnumerableProperty(span, ISOLATION_SCOPE_ON_START_SPAN_FIELD, makeWeakRef(isolationScope)); - addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, makeWeakRef(scope)); + // We don't wrap the scope with a WeakRef here because webkit aggressively garbage collects + // and scopes are not held in memory for long periods of time. + addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, scope); } } @@ -27,7 +29,7 @@ export function getCapturedScopesOnSpan(span: Span): { scope?: Scope; isolationS const spanWithScopes = span as SpanWithScopes; return { - scope: derefWeakRef(spanWithScopes[SCOPE_ON_START_SPAN_FIELD]), + scope: spanWithScopes[SCOPE_ON_START_SPAN_FIELD], isolationScope: derefWeakRef(spanWithScopes[ISOLATION_SCOPE_ON_START_SPAN_FIELD]), }; } diff --git a/packages/core/test/lib/tracing/utils.test.ts b/packages/core/test/lib/tracing/utils.test.ts index 6871f13a4698..c72c7a09109f 100644 --- a/packages/core/test/lib/tracing/utils.test.ts +++ b/packages/core/test/lib/tracing/utils.test.ts @@ -44,16 +44,15 @@ describe('tracing utils', () => { expect(retrieved.isolationScope).toBeUndefined(); }); - it('uses WeakRef for both scopes', () => { + it('uses WeakRef only for isolation scope, stores scope directly', () => { const span = createMockSpan(); const scope = new Scope(); const isolationScope = new Scope(); setCapturedScopesOnSpan(span, scope, isolationScope); - // Check that both scopes are wrapped with WeakRef const spanWithScopes = span as any; - expect(spanWithScopes._sentryScope).toBeInstanceOf(WeakRef); + expect(spanWithScopes._sentryScope).toBe(scope); expect(spanWithScopes._sentryIsolationScope).toBeInstanceOf(WeakRef); // Verify we can still retrieve the scopes @@ -90,38 +89,33 @@ describe('tracing utils', () => { } }); - it('handles WeakRef deref returning undefined gracefully', () => { + it('handles WeakRef deref returning undefined gracefully for isolation scope', () => { const span = createMockSpan(); + const scope = new Scope(); + const isolationScope = new Scope(); + + setCapturedScopesOnSpan(span, scope, isolationScope); - // Mock WeakRef.deref to return undefined (simulating garbage collection) + // Mock isolation scope WeakRef to return undefined (simulating garbage collection) const spanWithScopes = span as any; - const mockScopeWeakRef = { - deref: vi.fn().mockReturnValue(undefined), - }; const mockIsolationScopeWeakRef = { deref: vi.fn().mockReturnValue(undefined), }; - spanWithScopes._sentryScope = mockScopeWeakRef; spanWithScopes._sentryIsolationScope = mockIsolationScopeWeakRef; const retrieved = getCapturedScopesOnSpan(span); - expect(retrieved.scope).toBeUndefined(); + expect(retrieved.scope).toBe(scope); expect(retrieved.isolationScope).toBeUndefined(); - expect(mockScopeWeakRef.deref).toHaveBeenCalled(); expect(mockIsolationScopeWeakRef.deref).toHaveBeenCalled(); }); - it('handles corrupted WeakRef objects gracefully', () => { + it('handles corrupted WeakRef objects gracefully for isolation scope', () => { const span = createMockSpan(); + const scope = new Scope(); - // Set up corrupted WeakRefs that throw on deref const spanWithScopes = span as any; - spanWithScopes._sentryScope = { - deref: vi.fn().mockImplementation(() => { - throw new Error('WeakRef deref failed'); - }), - }; + spanWithScopes._sentryScope = scope; spanWithScopes._sentryIsolationScope = { deref: vi.fn().mockImplementation(() => { throw new Error('WeakRef deref failed'); @@ -129,7 +123,7 @@ describe('tracing utils', () => { }; const retrieved = getCapturedScopesOnSpan(span); - expect(retrieved.scope).toBeUndefined(); + expect(retrieved.scope).toBe(scope); expect(retrieved.isolationScope).toBeUndefined(); }); From 6619ea61f7c5794f6611c2c11ea90fb4c5181abe Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 29 May 2026 14:03:37 +0200 Subject: [PATCH 3/4] fixup! fix(core): Use WeakRef for Span-Scope circular references --- packages/core/src/utils/spanUtils.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 039031db963b..29d1d7244abb 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -353,7 +353,6 @@ type SpanWithPotentialChildren = Span & { /** * Adds an opaque child span reference to a span. - * Uses WeakRef to allow child spans to be garbage collected when no longer needed. */ export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void { // We store the root span reference on the child span @@ -361,12 +360,16 @@ export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: S const rootSpan = span[ROOT_SPAN_FIELD] || span; addNonEnumerableProperty(childSpan as SpanWithPotentialChildren, ROOT_SPAN_FIELD, rootSpan); - const ref = makeWeakRef(childSpan); - - if (span[CHILD_SPANS_FIELD]) { - span[CHILD_SPANS_FIELD].add(ref); + const children = span[CHILD_SPANS_FIELD]; + if (children) { + for (const ref of children) { + if (derefWeakRef(ref) === childSpan) { + return; + } + } + children.add(makeWeakRef(childSpan)); } else { - addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([ref])); + addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([makeWeakRef(childSpan)])); } } From 5df54e790eecb4459aae7605ccf301a62969dc15 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 29 May 2026 18:37:57 +0200 Subject: [PATCH 4/4] fixup! fix(core): Use WeakRef for Span-Scope circular references --- packages/core/src/utils/spanUtils.ts | 39 ++++++-------------- packages/core/test/lib/tracing/trace.test.ts | 7 ++-- 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 29d1d7244abb..9f495ef7b30e 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -1,5 +1,3 @@ -/* eslint-disable max-lines */ - import { getAsyncContextStrategy } from '../asyncContext'; import type { RawAttributes } from '../attributes'; import { serializeAttributes } from '../attributes'; @@ -32,8 +30,6 @@ import { timestampInSeconds } from '../utils/time'; import { generateSentryTraceHeader, generateTraceparentHeader } from '../utils/tracing'; import { consoleSandbox } from './debug-logger'; import { _getSpanForScope } from './spanOnScope'; -import type { MaybeWeakRef } from './weakRef'; -import { derefWeakRef, makeWeakRef } from './weakRef'; // These are aligned with OpenTelemetry trace flags export const TRACE_FLAG_NONE = 0x0; @@ -347,7 +343,7 @@ const CHILD_SPANS_FIELD = '_sentryChildSpans'; const ROOT_SPAN_FIELD = '_sentryRootSpan'; type SpanWithPotentialChildren = Span & { - [CHILD_SPANS_FIELD]?: Set>; + [CHILD_SPANS_FIELD]?: Set; [ROOT_SPAN_FIELD]?: Span; }; @@ -360,29 +356,19 @@ export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: S const rootSpan = span[ROOT_SPAN_FIELD] || span; addNonEnumerableProperty(childSpan as SpanWithPotentialChildren, ROOT_SPAN_FIELD, rootSpan); - const children = span[CHILD_SPANS_FIELD]; - if (children) { - for (const ref of children) { - if (derefWeakRef(ref) === childSpan) { - return; - } - } - children.add(makeWeakRef(childSpan)); + // We store a list of child spans on the parent span + // We need this for `getSpanDescendants()` to work + if (span[CHILD_SPANS_FIELD]) { + span[CHILD_SPANS_FIELD].add(childSpan); } else { - addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([makeWeakRef(childSpan)])); + addNonEnumerableProperty(span, CHILD_SPANS_FIELD, new Set([childSpan])); } } /** This is only used internally by Idle Spans. */ export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void { - const children = span[CHILD_SPANS_FIELD]; - if (children) { - for (const ref of children) { - if (derefWeakRef(ref) === childSpan) { - children.delete(ref); - break; - } - } + if (span[CHILD_SPANS_FIELD]) { + span[CHILD_SPANS_FIELD].delete(childSpan); } } @@ -399,12 +385,9 @@ export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] { // We want to ignore unsampled spans (e.g. non recording spans) } else if (spanIsSampled(span)) { resultSet.add(span); - const childRefs = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : []; - for (const ref of childRefs) { - const childSpan = derefWeakRef(ref); - if (childSpan) { - addSpanChildren(childSpan); - } + const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : []; + for (const childSpan of childSpans) { + addSpanChildren(childSpan); } } } diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 451ed38c706f..0d0e652e64aa 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -13,7 +13,6 @@ import { spanToJSON, withScope, } from '../../../src'; -import { derefWeakRef } from '../../../src/utils/weakRef'; import { getAsyncContextStrategy } from '../../../src/asyncContext'; import { continueTrace, @@ -787,7 +786,7 @@ describe('startSpan', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { startSpan({ name: 'inner' }, innerSpan => { - const childSpans = Array.from(outerSpan._sentryChildSpans).map(derefWeakRef); + const childSpans = Array.from(outerSpan._sentryChildSpans); expect(childSpans).toContain(innerSpan); }); }); @@ -1336,7 +1335,7 @@ describe('startSpanManual', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { startSpanManual({ name: 'inner' }, innerSpan => { - const childSpans = Array.from(outerSpan._sentryChildSpans).map(derefWeakRef); + const childSpans = Array.from(outerSpan._sentryChildSpans); expect(childSpans).toContain(innerSpan); }); }); @@ -1803,7 +1802,7 @@ describe('startInactiveSpan', () => { expect.assertions(1); startSpan({ name: 'outer' }, (outerSpan: any) => { const innerSpan = startInactiveSpan({ name: 'inner' }); - const childSpans = Array.from(outerSpan._sentryChildSpans).map(derefWeakRef); + const childSpans = Array.from(outerSpan._sentryChildSpans); expect(childSpans).toContain(innerSpan); }); });