Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,9 @@ function runTestFile(path, filesWatcher, opts) {
finished(child.stdout, { __proto__: null, signal: t.signal }),
]);

// Close readline interface to prevent memory leak
rl.close();

if (watchMode) {
filesWatcher.runningProcesses.delete(path);
filesWatcher.runningSubtests.delete(path);
Expand Down Expand Up @@ -506,7 +509,7 @@ function watchFiles(testFiles, opts) {
}

// Watch for changes in current filtered files
watcher.on('changed', ({ owners, eventType }) => {
const onChanged = ({ owners, eventType }) => {
if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) {
const updatedTestFiles = createTestFileList(opts.globPatterns, opts.cwd);
const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x));
Expand Down Expand Up @@ -547,19 +550,29 @@ function watchFiles(testFiles, opts) {
triggerUncaughtException(error, true /* fromPromise */);
}));
}
});
};

watcher.on('changed', onChanged);

// Cleanup function to remove event listener and prevent memory leak
const cleanup = () => {
watcher.removeListener('changed', onChanged);
opts.root.harness.watching = false;
opts.root.postRun();
};

if (opts.signal) {
kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
opts.signal.addEventListener(
'abort',
() => {
opts.root.harness.watching = false;
opts.root.postRun();
},
cleanup,
{ __proto__: null, once: true, [kResistStopPropagation]: true },
);
}

// Expose cleanup method for proper resource management
filesWatcher.cleanup = cleanup;

return filesWatcher;
}

Expand Down
99 changes: 99 additions & 0 deletions test/parallel/test-runner-memory-leak.js

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is passing on main, it's at best unrelated to this PR, please remove it

Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
'use strict';

// Regression test for memory leaks in test runner
// Without the fix, these tests could hang or accumulate resources

const common = require('../common');
const assert = require('assert');
const { spawn } = require('child_process');
const { writeFileSync } = require('fs');
const { join } = require('path');
const tmpdir = require('../common/tmpdir');

tmpdir.refresh();

// Test 1: Verify readline interface cleanup
// Without fix: readline.Interface remains open when child has stderr output
// This could cause the process to hang or resources to leak
{
const testFile = join(tmpdir.path, 'test-with-stderr.js');
writeFileSync(
testFile,
`
const test = require('node:test');
test('test with stderr', () => {
process.stderr.write('line1\\n');
process.stderr.write('line2\\n');
});
`
);

const child = spawn(process.execPath, [
'--test',
'--test-isolation=process',
testFile,
]);

let stdout = '';
child.stdout.on('data', (chunk) => {
stdout += chunk;
});

child.on(
'close',
common.mustCall((code) => {
assert.strictEqual(code, 0);
assert.match(stdout, /tests 1/);
assert.match(stdout, /pass 1/);
})
);

// Without fix: process might not exit cleanly due to unclosed readline
const timeout = setTimeout(() => {
if (!child.killed) {
child.kill();
assert.fail('Process did not exit within timeout');
}
}, 30000);
timeout.unref();
}

// Test 2: Verify watch mode can be stopped cleanly
// Without fix: watch mode listener might not be removed on abort
{
const testFile = join(tmpdir.path, 'test-watch.js');
writeFileSync(
testFile,
`
const test = require('node:test');
test('watch test', () => {});
`
);

const child = spawn(process.execPath, ['--test', '--watch', testFile]);

let stdout = '';
child.stdout.on('data', (chunk) => {
stdout += chunk;
if (stdout.includes('pass 1')) {
child.kill('SIGTERM');
}
});

child.on(
'close',
common.mustCall(() => {
assert.match(stdout, /tests 1/);
assert.match(stdout, /pass 1/);
})
);

// Without fix: process might not respond to SIGTERM properly
const timeout = setTimeout(() => {
if (!child.killed) {
child.kill('SIGKILL');
assert.fail('Watch mode did not exit cleanly on SIGTERM');
}
}, 10000);
timeout.unref();
}