Skip to content

Commit

Permalink
fix(profiling-node): Guard invocation of native profiling methods (#1…
Browse files Browse the repository at this point in the history
…4676)

Add a guard to check that the native `startProfiling` and
`stopProfiling` APIs are actually available and functions. This came up
in #14561 where it seems like they're not always loaded correctly. For
reasons unknown to me.

ref #14561
  • Loading branch information
Lms24 authored Dec 12, 2024
1 parent 63476fa commit 5af2833
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 11 deletions.
12 changes: 12 additions & 0 deletions packages/profiling-node/src/cpu_profiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ class Bindings implements V8CpuProfilerBindings {
return;
}

if (typeof PrivateCpuProfilerBindings.startProfiling !== 'function') {
DEBUG_BUILD &&
logger.log('[Profiling] Native startProfiling function is not available, ignoring call to startProfiling.');
return;
}

return PrivateCpuProfilerBindings.startProfiling(name);
}

Expand All @@ -187,6 +193,12 @@ class Bindings implements V8CpuProfilerBindings {
return null;
}

if (typeof PrivateCpuProfilerBindings.stopProfiling !== 'function') {
DEBUG_BUILD &&
logger.log('[Profiling] Native stopProfiling function is not available, ignoring call to stopProfiling.');
return null;
}

return PrivateCpuProfilerBindings.stopProfiling(
name,
format as unknown as any,
Expand Down
6 changes: 3 additions & 3 deletions packages/profiling-node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ export interface RawChunkCpuProfile extends BaseProfile {
}

export interface PrivateV8CpuProfilerBindings {
startProfiling(name: string): void;
startProfiling?: (name: string) => void;

stopProfiling(
stopProfiling?(
name: string,
format: ProfileFormat.THREAD,
threadId: number,
collectResources: boolean,
): RawThreadCpuProfile | null;
stopProfiling(
stopProfiling?(
name: string,
format: ProfileFormat.CHUNK,
threadId: number,
Expand Down
39 changes: 31 additions & 8 deletions packages/profiling-node/test/cpu_profiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,28 +69,28 @@ const assertValidMeasurements = (measurement: RawThreadCpuProfile['measurements'

describe('Private bindings', () => {
it('does not crash if collect resources is false', async () => {
PrivateCpuProfilerBindings.startProfiling('profiled-program');
PrivateCpuProfilerBindings.startProfiling!('profiled-program');
await wait(100);
expect(() => {
const profile = PrivateCpuProfilerBindings.stopProfiling('profiled-program', 0, 0, false);
const profile = PrivateCpuProfilerBindings.stopProfiling!('profiled-program', 0, 0, false);
if (!profile) throw new Error('No profile');
}).not.toThrow();
});

it('throws if invalid format is supplied', async () => {
PrivateCpuProfilerBindings.startProfiling('profiled-program');
PrivateCpuProfilerBindings.startProfiling!('profiled-program');
await wait(100);
expect(() => {
const profile = PrivateCpuProfilerBindings.stopProfiling('profiled-program', Number.MAX_SAFE_INTEGER, 0, false);
const profile = PrivateCpuProfilerBindings.stopProfiling!('profiled-program', Number.MAX_SAFE_INTEGER, 0, false);
if (!profile) throw new Error('No profile');
}).toThrow('StopProfiling expects a valid format type as second argument.');
});

it('collects resources', async () => {
PrivateCpuProfilerBindings.startProfiling('profiled-program');
PrivateCpuProfilerBindings.startProfiling!('profiled-program');
await wait(100);

const profile = PrivateCpuProfilerBindings.stopProfiling('profiled-program', 0, 0, true);
const profile = PrivateCpuProfilerBindings.stopProfiling!('profiled-program', 0, 0, true);
if (!profile) throw new Error('No profile');

expect(profile.resources.length).toBeGreaterThan(0);
Expand All @@ -104,10 +104,10 @@ describe('Private bindings', () => {
});

it('does not collect resources', async () => {
PrivateCpuProfilerBindings.startProfiling('profiled-program');
PrivateCpuProfilerBindings.startProfiling!('profiled-program');
await wait(100);

const profile = PrivateCpuProfilerBindings.stopProfiling('profiled-program', 0, 0, false);
const profile = PrivateCpuProfilerBindings.stopProfiling!('profiled-program', 0, 0, false);
if (!profile) throw new Error('No profile');

expect(profile.resources.length).toBe(0);
Expand Down Expand Up @@ -337,4 +337,27 @@ describe('Profiler bindings', () => {
const hasDeoptimizedFrame = profile.frames.some(f => f.deopt_reasons && f.deopt_reasons.length > 0);
expect(hasDeoptimizedFrame).toBe(true);
});

it('does not crash if the native startProfiling function is not available', async () => {
const original = PrivateCpuProfilerBindings.startProfiling;
PrivateCpuProfilerBindings.startProfiling = undefined;

expect(() => {
CpuProfilerBindings.startProfiling('profiled-program');
}).not.toThrow();

PrivateCpuProfilerBindings.startProfiling = original;
});

it('does not crash if the native stopProfiling function is not available', async () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const original = PrivateCpuProfilerBindings.stopProfiling;
PrivateCpuProfilerBindings.stopProfiling = undefined;

expect(() => {
CpuProfilerBindings.stopProfiling('profiled-program', 0);
}).not.toThrow();

PrivateCpuProfilerBindings.stopProfiling = original;
});
});

0 comments on commit 5af2833

Please sign in to comment.