Skip to content

Commit 3815492

Browse files
authored
fix(profiling): Disable profiling in worker threads (#20040)
The native CPU profiler's sampling thread can race with V8's GC in worker threads, causing heap corruption and ~40-60% crash rate under allocation pressure. This PR adds a JS-side guard while a long-term native addon should be added separately. - Adds isMainThread guard in ContinuousProfiler.initialize() to skip profiler startup in worker threads - Adds isMainThread guard in maybeProfileSpan() to prevent legacy span profiling in worker threads - Updates worker thread tests to verify profiling is a no-op across all profiling modes closes #20029 repro https://github.com/chargome/repro.JS-2019
1 parent 61edc25 commit 3815492

File tree

3 files changed

+74
-35
lines changed

3 files changed

+74
-35
lines changed

packages/profiling-node/src/integration.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@sentry/core';
1515
import type { NodeClient, NodeOptions } from '@sentry/node';
1616
import { CpuProfilerBindings, ProfileFormat, type RawThreadCpuProfile } from '@sentry-internal/node-cpu-profiler';
17+
import { isMainThread } from 'worker_threads';
1718
import { DEBUG_BUILD } from './debug-build';
1819
import { NODE_MAJOR } from './nodeVersion';
1920
import { MAX_PROFILE_DURATION_MS, maybeProfileSpan, stopSpanProfile } from './spanProfileUtils';
@@ -62,6 +63,14 @@ class ContinuousProfiler {
6263
* @param client
6364
*/
6465
public initialize(client: NodeClient): void {
66+
if (!isMainThread) {
67+
DEBUG_BUILD &&
68+
debug.warn(
69+
'[Profiling] nodeProfilingIntegration() does not support worker threads — profiling will be disabled for this thread.',
70+
);
71+
return;
72+
}
73+
6574
this._client = client;
6675
const options = client.getOptions();
6776

packages/profiling-node/src/spanProfileUtils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { CustomSamplingContext, Span } from '@sentry/core';
33
import { debug, spanIsSampled, spanToJSON, uuid4 } from '@sentry/core';
44
import type { NodeClient } from '@sentry/node';
55
import { CpuProfilerBindings, type RawThreadCpuProfile } from '@sentry-internal/node-cpu-profiler';
6+
import { isMainThread } from 'worker_threads';
67
import { DEBUG_BUILD } from './debug-build';
78
import { isValidSampleRate } from './utils';
89

@@ -17,6 +18,13 @@ export function maybeProfileSpan(
1718
span: Span,
1819
customSamplingContext?: CustomSamplingContext,
1920
): string | undefined {
21+
// Profiling is not supported in worker threads as the native CPU profiler's
22+
// sampling thread can race with V8's GC across isolates, causing heap corruption.
23+
if (!isMainThread) {
24+
DEBUG_BUILD && debug.log('[Profiling] Skipping span profiling in worker thread.');
25+
return;
26+
}
27+
2028
// profilesSampleRate is multiplied with tracesSampleRate to get the final sampling rate. We dont perform
2129
// the actual multiplication to get the final rate, but we discard the profile if the span was sampled,
2230
// so anything after this block from here is based on the span sampling.
Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ProfilingIntegration, Transport } from '@sentry/core';
22
import * as Sentry from '@sentry/node';
3-
import { expect, it, vi } from 'vitest';
3+
import { CpuProfilerBindings } from '@sentry-internal/node-cpu-profiler';
4+
import { afterEach, describe, expect, it, vi } from 'vitest';
45
import { _nodeProfilingIntegration } from '../src/integration';
56

67
// Mock the modules before the import, so that the value is initialized before the module is loaded
@@ -12,7 +13,7 @@ vi.mock('worker_threads', () => {
1213
});
1314
vi.setConfig({ testTimeout: 10_000 });
1415

15-
function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] {
16+
function makeClient(options: Partial<Sentry.NodeOptions> = {}): [Sentry.NodeClient, Transport] {
1617
const integration = _nodeProfilingIntegration();
1718
const client = new Sentry.NodeClient({
1819
stackParser: Sentry.defaultStackParser,
@@ -28,48 +29,69 @@ function makeContinuousProfilingClient(): [Sentry.NodeClient, Transport] {
2829
return undefined;
2930
},
3031
}),
32+
...options,
3133
});
3234

3335
return [client, client.getTransport() as Transport];
3436
}
3537

36-
it('worker threads context', () => {
37-
const [client, transport] = makeContinuousProfilingClient();
38-
Sentry.setCurrentClient(client);
39-
client.init();
38+
describe('worker threads', () => {
39+
afterEach(() => {
40+
vi.restoreAllMocks();
41+
});
42+
43+
it('does not start continuous profiling in worker threads', () => {
44+
const startProfilingSpy = vi.spyOn(CpuProfilerBindings, 'startProfiling');
45+
46+
const [client] = makeClient();
47+
Sentry.setCurrentClient(client);
48+
client.init();
49+
50+
const integration = client.getIntegrationByName<ProfilingIntegration<any>>('ProfilingIntegration');
51+
if (!integration) {
52+
throw new Error('Profiling integration not found');
53+
}
4054

41-
const transportSpy = vi.spyOn(transport, 'send').mockReturnValue(Promise.resolve({}));
55+
// Calling start should be a no-op in a worker thread
56+
integration._profiler.start();
4257

43-
const nonProfiledTransaction = Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' });
44-
nonProfiledTransaction.end();
58+
const transaction = Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' });
59+
transaction.end();
4560

46-
expect(transportSpy.mock.calls?.[0]?.[0]?.[1]?.[0]?.[1]).not.toMatchObject({
47-
contexts: {
48-
profile: {},
49-
},
61+
// The native profiler should never have been called
62+
expect(startProfilingSpy).not.toHaveBeenCalled();
63+
64+
integration._profiler.stop();
5065
});
5166

52-
const integration = client.getIntegrationByName<ProfilingIntegration<any>>('ProfilingIntegration');
53-
if (!integration) {
54-
throw new Error('Profiling integration not found');
55-
}
56-
57-
integration._profiler.start();
58-
const profiledTransaction = Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' });
59-
profiledTransaction.end();
60-
integration._profiler.stop();
61-
62-
expect(transportSpy.mock.calls?.[1]?.[0]?.[1]?.[0]?.[1]).toMatchObject({
63-
contexts: {
64-
trace: {
65-
data: expect.objectContaining({
66-
['thread.id']: '9999',
67-
['thread.name']: 'worker',
68-
}),
69-
},
70-
profile: {
71-
profiler_id: expect.any(String),
72-
},
73-
},
67+
it('does not start span profiling in worker threads', () => {
68+
const startProfilingSpy = vi.spyOn(CpuProfilerBindings, 'startProfiling');
69+
70+
const [client] = makeClient({ profilesSampleRate: 1 });
71+
Sentry.setCurrentClient(client);
72+
client.init();
73+
74+
const transaction = Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' });
75+
transaction.end();
76+
77+
// The native profiler should never have been called even with profilesSampleRate set
78+
expect(startProfilingSpy).not.toHaveBeenCalled();
79+
});
80+
81+
it('does not start trace lifecycle profiling in worker threads', () => {
82+
const startProfilingSpy = vi.spyOn(CpuProfilerBindings, 'startProfiling');
83+
84+
const [client] = makeClient({
85+
profileSessionSampleRate: 1.0,
86+
profileLifecycle: 'trace',
87+
});
88+
Sentry.setCurrentClient(client);
89+
client.init();
90+
91+
const transaction = Sentry.startInactiveSpan({ forceTransaction: true, name: 'profile_hub' });
92+
transaction.end();
93+
94+
// The native profiler should never have been called
95+
expect(startProfilingSpy).not.toHaveBeenCalled();
7496
});
7597
});

0 commit comments

Comments
 (0)