Skip to content

Commit f5ac627

Browse files
authored
fix(v8/node): Enforce that ContextLines integration does not leave open file handles (#14997)
v8 backport of #14995
1 parent 286f6b0 commit f5ac627

File tree

12 files changed

+292
-6
lines changed

12 files changed

+292
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
1212

13-
Work in this release was contributed by @HHK1. Thank you for your contribution!
13+
Work in this release was contributed by @HHK1 and @mstrokin. Thank you for your contribution!
1414

1515
## 8.48.0
1616

dev-packages/node-integration-tests/suites/contextLines/test.ts renamed to dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { join } from 'path';
2-
import { conditionalTest } from '../../utils';
3-
import { createRunner } from '../../utils/runner';
2+
import { conditionalTest } from '../../../utils';
3+
import { createRunner } from '../../../utils/runner';
44

55
conditionalTest({ min: 18 })('ContextLines integration in ESM', () => {
66
test('reads encoded context lines from filenames with spaces', done => {
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import * as Sentry from '@sentry/node';
2+
3+
export function captureException(i: number): void {
4+
Sentry.captureException(new Error(`error in loop ${i}`));
5+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { captureException } from './nested-file';
2+
3+
export function runSentry(): void {
4+
for (let i = 0; i < 10; i++) {
5+
captureException(i);
6+
}
7+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { execSync } from 'node:child_process';
2+
import * as path from 'node:path';
3+
4+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
5+
import * as Sentry from '@sentry/node';
6+
7+
Sentry.init({
8+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
9+
release: '1.0',
10+
transport: loggingTransport,
11+
});
12+
13+
import { runSentry } from './other-file';
14+
15+
runSentry();
16+
17+
const lsofOutput = execSync(`lsof -p ${process.pid}`, { encoding: 'utf8' });
18+
const lsofTable = lsofOutput.split('\n');
19+
const mainPath = __dirname.replace(`${path.sep}suites${path.sep}contextLines${path.sep}memory-leak`, '');
20+
const numberOfLsofEntriesWithMainPath = lsofTable.filter(entry => entry.includes(mainPath));
21+
22+
// There should only be a single entry with the main path, otherwise we are leaking file handles from the
23+
// context lines integration.
24+
if (numberOfLsofEntriesWithMainPath.length > 1) {
25+
// eslint-disable-next-line no-console
26+
console.error('Leaked file handles detected');
27+
// eslint-disable-next-line no-console
28+
console.error(lsofTable);
29+
process.exit(1);
30+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { conditionalTest } from '../../../utils';
2+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
3+
4+
conditionalTest({ min: 18 })('ContextLines integration in CJS', () => {
5+
afterAll(() => {
6+
cleanupChildProcesses();
7+
});
8+
9+
// Regression test for: https://github.com/getsentry/sentry-javascript/issues/14892
10+
test('does not leak open file handles', done => {
11+
createRunner(__dirname, 'scenario.ts')
12+
.expectN(10, {
13+
event: {},
14+
})
15+
.start(done);
16+
});
17+
});

dev-packages/node-integration-tests/test.txt

Lines changed: 213 additions & 0 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)