test(pty): release the live pty before the next fd-reuse case

The new Linux write case killed its live pty without awaiting the exit, so that
master was still open when the next case spawned. Linux hands out the lowest
free descriptor, so the retired pty got N+1 while the leaked N closed during the
400ms settle -- and `does not name a live pty foreground process off the retired
descriptor` then saw its live pty take N, failing its own reuse premise by
exactly one (`expected 22 to be 23` on CI, reproduced as 19/20 in a container).

Test isolation, not a Linux behavior difference: with the descriptor released
the case passes 8/8. Await the exit like the resize case above already does, and
switch the payload command to `sleep 1` -- the line discipline echoes a leaked
write back on its own, so nothing has to read it. Note the constraint on the
block so the next case does not repeat it.

The premise assertion stays as it is. It compares two observed descriptors, not
a hardcoded number; relaxing it to "differs from the retired one" would let both
reuse cases pass vacuously on a kernel that never reissues, which is the whole
thing they exist to demonstrate.

Verified with vitest under node:24-bookworm against a source-built patched
node-pty: 7 passed, 8 runs, no flake. Negative check in the same container --
reverting the `_close()` -> `dispose()` hook fails the case with
`expected 'leak\r\n' not to contain 'leak'`, so the rewritten assertion is still
load-bearing.
This commit is contained in:
Neil
2026-09-02 01:48:39 -07:00
parent 8f46541c9d
commit e818172708
@@ -108,6 +108,11 @@ describeOnPosix('node-pty master fd retirement', () => {
// Linux frees the master synchronously enough that the very next pty is handed the
// same descriptor number every time, which makes the reuse hazard directly
// observable rather than a race to reproduce.
//
// Which is also the constraint on writing a case here: the kernel hands out the
// lowest free number, so a case that returns while its live pty is still open
// leaks that descriptor into the next case's premise as an off-by-one. Await the
// exit, never a fixed sleep.
const describeOnLinux = process.platform === 'linux' ? describe : describe.skip
describeOnLinux('node-pty master fd reuse', () => {
@@ -141,7 +146,7 @@ describeOnLinux('node-pty master fd reuse', () => {
it('cannot write into a live pty handed the retired descriptor number', async () => {
const { term: retired, spawnFd } = await retiredPty('exit 0', { destroy: false })
const live = spawnPty('read -r line; printf "got:%s" "$line"')
const live = spawnPty('sleep 1')
let output = ''
live.onData((data) => {
output += data
@@ -149,11 +154,19 @@ describeOnLinux('node-pty master fd reuse', () => {
try {
expect(masterFd(live)).toBe(spawnFd)
// Pre-patch this fs.write reached `live`'s master and the tty echoed it
// back: a retired pane's bytes landing in an unrelated terminal.
// Pre-patch this fs.write reached `live`'s master, and the line discipline
// echoed it straight back: a retired pane's bytes landing in an unrelated
// terminal. Nothing has to read them for the leak to be observable.
writeStream(retired).write('leak\r')
await new Promise<void>((resolve) => setTimeout(resolve, 300))
// Await the exit rather than sleeping: a fixed wait leaves this descriptor
// open into the next case, whose `expect(masterFd(live)).toBe(spawnFd)`
// premise then sees the kernel hand out the lower number this pty was still
// holding. That is what broke `node 24 1/8` on Linux, where alone among the
// platforms these cases actually run.
await new Promise<void>((resolve) => {
live.onExit(() => resolve())
})
expect(output).not.toContain('leak')
} finally {
live.kill()