Skip to content

Commit d8c3c65

Browse files
authored
fix: revert grandchild fix for now (#140)
* test: fix up grandchild tests They passed on main before, now they actually fail before the destroy fix. * chore: revert fix for now
1 parent 20e4117 commit d8c3c65

6 files changed

Lines changed: 29 additions & 62 deletions

File tree

src/main.ts

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ export {NonZeroExitError, normalizeSpawnCommand};
1717

1818
const LINE_SEPARATOR_REGEX = /\r?\n/;
1919

20-
const pipedStreams = new WeakSet<Readable>();
21-
2220
export interface Output {
2321
stderr: string;
2422
stdout: string;
@@ -336,7 +334,6 @@ export class ExecProcess implements Result {
336334

337335
this._process = handle;
338336
handle.once('error', this._onError);
339-
handle.once('exit', this._onExit);
340337
handle.once('close', this._onClose);
341338

342339
if (handle.stdin) {
@@ -345,11 +342,7 @@ export class ExecProcess implements Result {
345342
if (typeof stdin === 'string') {
346343
handle.stdin.end(stdin);
347344
} else {
348-
const src = stdin?.process?.stdout;
349-
if (src) {
350-
src.pipe(handle.stdin);
351-
pipedStreams.add(src);
352-
}
345+
stdin?.process?.stdout?.pipe(handle.stdin);
353346
}
354347
}
355348
}
@@ -373,28 +366,6 @@ export class ExecProcess implements Result {
373366
this._thrownError = err;
374367
};
375368

376-
protected _onExit = (): void => {
377-
// Node emits 'exit' before stdio streams have drained. Use setImmediate
378-
// to let buffered data flow through before destroying the streams.
379-
// If grandchild processes hold the pipe fds open, they would never fire
380-
// 'close', so we destroy here to unblock readStream and combineStreams.
381-
const out =
382-
this._streamOut && !pipedStreams.has(this._streamOut)
383-
? this._streamOut
384-
: undefined;
385-
const err =
386-
this._streamErr && !pipedStreams.has(this._streamErr)
387-
? this._streamErr
388-
: undefined;
389-
if (!out && !err) {
390-
return;
391-
}
392-
setImmediate(() => {
393-
out?.destroy();
394-
err?.destroy();
395-
});
396-
};
397-
398369
protected _onClose = (): void => {
399370
if (this._resolveClose) {
400371
this._resolveClose();

src/test/main_test.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -367,15 +367,13 @@ if (!isWindows) {
367367
}
368368
});
369369

370-
test('resolves when grandchild holds piped stdout open', async () => {
370+
test.skip('resolves when grandchild holds piped stdout open', async () => {
371371
const dir = fs.mkdtempSync(
372372
path.join(os.tmpdir(), 'tinyexec-grandchild-')
373373
);
374374
const runnerScript = path.join(dir, 'runner.mjs');
375375
const distPath = JSON.stringify(path.join(distDir, 'main.mjs'));
376-
const fixturePath = JSON.stringify(
377-
path.join(fixturesDir, 'grandchild.mjs')
378-
);
376+
const fixturePath = JSON.stringify(path.join(fixturesDir, 'child.mjs'));
379377

380378
fs.writeFileSync(
381379
runnerScript,
@@ -399,19 +397,19 @@ if (!isWindows) {
399397
expect(parsed.exitCode).toBe(0);
400398
expect(parsed.stdout).toBe('output\n');
401399
} finally {
402-
spawnSync('pkill', ['-f', 'tinyexec-test-grandchild']);
400+
spawnSync('pkill', ['-f', 'grandchild.mjs']);
403401
fs.rmSync(dir, {recursive: true, force: true});
404402
}
405403
});
406404

407-
test('iterator completes when grandchild holds piped stdout open', async () => {
405+
test.skip('iterator completes when grandchild holds piped stdout open', async () => {
408406
const dir = fs.mkdtempSync(
409407
path.join(os.tmpdir(), 'tinyexec-grandchild-')
410408
);
411409
const runnerScript = path.join(dir, 'runner.mjs');
412410
const distPath = JSON.stringify(path.join(distDir, 'main.mjs'));
413411
const fixturePath = JSON.stringify(
414-
path.join(fixturesDir, 'grandchild_multiline.mjs')
412+
path.join(fixturesDir, 'child_multiline.mjs')
415413
);
416414

417415
fs.writeFileSync(
@@ -438,7 +436,7 @@ if (!isWindows) {
438436
const parsed = JSON.parse(proc.stdout.trim());
439437
expect(parsed).toEqual(['line1', 'line2']);
440438
} finally {
441-
spawnSync('pkill', ['-f', 'tinyexec-test-grandchild']);
439+
spawnSync('pkill', ['-f', 'grandchild.mjs']);
442440
fs.rmSync(dir, {recursive: true, force: true});
443441
}
444442
});

test/fixtures/child.mjs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import {spawn} from 'node:child_process';
2+
import path from 'node:path';
3+
4+
// Spawn a grandchild that inherits our piped stdout fd (fd 1), simulating
5+
// tsserver inheriting eslint's piped streams. The grandchild outlives us and
6+
// holds the pipe open.
7+
const grandchild = path.join(import.meta.dirname, 'grandchild.mjs');
8+
spawn(process.argv[0], [grandchild], {stdio: ['ignore', 1, 'ignore']});
9+
10+
console.log('output');
11+
process.exit(0);

test/fixtures/child_multiline.mjs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import {spawn} from 'node:child_process';
2+
import path from 'node:path';
3+
4+
const grandchild = path.join(import.meta.dirname, 'grandchild.mjs');
5+
spawn(process.argv[0], [grandchild], {stdio: ['ignore', 1, 'ignore']});
6+
7+
console.log('line1');
8+
console.log('line2');
9+
process.exit(0);

test/fixtures/grandchild.mjs

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,2 @@
1-
import {spawn} from 'node:child_process';
2-
3-
// Spawn a grandchild that inherits the piped stdout fd, simulating
4-
// tsserver inheriting eslint's piped streams. Short timeout to avoid
5-
// blocking test teardown.
6-
spawn(
7-
process.argv[0],
8-
['-e', 'setTimeout(() => void 0, 3000)'],
9-
{stdio: ['ignore', 1, 'ignore']}
10-
);
11-
12-
console.log('output');
13-
process.exit(0);
1+
// Run for longer than the test timeout
2+
setTimeout(() => {}, 30000);

test/fixtures/grandchild_multiline.mjs

Lines changed: 0 additions & 11 deletions
This file was deleted.

0 commit comments

Comments
 (0)