From d69919669285979650ebbddde7da2e71c6210b24 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 11:18:34 +0000 Subject: [PATCH] node:fs: Dir.close must not close an fd it did not open Dir in the path-bound implementation never opens a descriptor of its own (opendir/opendirSync pass the sentinel 1 as the handle), so the `if (handle > 2) fs.closeSync(handle)` in close()/closeSync() is dead for legitimate use and only ever fires on a userland `new fs.Dir(fd, path)`, closing an unrelated fd by number. Node treats the handle as an opaque DirHandle and never touches an integer passed there. Drop the closeSync calls; #handle is now purely the open/closed marker. --- src/js/node/fs.ts | 13 ++++------ test/js/node/fs/dir.test.ts | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index d3e1ce3f98dd..0ea54353c2d3 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -1054,8 +1054,9 @@ const kAlreadyValidated = Symbol("kAlreadyValidated"); class Dir { /** - * `-1` when closed. stdio handles (0, 1, 2) don't actually get closed by - * {@link close} or {@link closeSync}. + * Open/closed state sentinel (`-1` once closed). This implementation is + * path-bound and never owns a real fd, so close() only flips this marker; + * it must not `fs.closeSync` a value the constructor was handed. */ #handle: number; #path: PathLike; @@ -1174,9 +1175,7 @@ class Dir { } #closeOp() { - const handle = this.#handle; - if (handle < 0) throw $ERR_DIR_CLOSED(); - if (handle > 2) fs.closeSync(handle); + if (this.#handle < 0) throw $ERR_DIR_CLOSED(); this.#handle = -1; } @@ -1190,10 +1189,8 @@ class Dir { } closeSync() { - const handle = this.#handle; - if (handle < 0) throw $ERR_DIR_CLOSED(); + if (this.#handle < 0) throw $ERR_DIR_CLOSED(); if (this.#pendingCount > 0) throw this.#dirConcurrentError(); - if (handle > 2) fs.closeSync(handle); this.#handle = -1; } diff --git a/test/js/node/fs/dir.test.ts b/test/js/node/fs/dir.test.ts index c4c383d4677c..dc54a840ffa6 100644 --- a/test/js/node/fs/dir.test.ts +++ b/test/js/node/fs/dir.test.ts @@ -1,4 +1,5 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; +import { tempDir } from "harness"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -241,3 +242,50 @@ describe("Dir explicit resource management", () => { await expect(dir[Symbol.asyncDispose]()).resolves.toBeUndefined(); }); }); + +// A Dir constructed directly with a raw integer must not treat that integer as +// an fd it owns: node's handle is an opaque DirHandle and calling close() on an +// integer throws there without touching any descriptor. +describe("new fs.Dir with a foreign integer handle", () => { + function openProbe(cwd: string) { + const fd = fs.openSync(path.join(cwd, "probe.txt"), "r"); + return { + fd, + [Symbol.dispose]() { + try { + fs.closeSync(fd); + } catch {} + }, + }; + } + + it("closeSync does not close the unrelated fd", () => { + using cwd = tempDir("dir-foreign-fd", { "probe.txt": "x" }); + using probe = openProbe(String(cwd)); + const d = new fs.Dir(probe.fd, String(cwd)); + try { + d.closeSync(); + } catch { + // node throws TypeError here; either way the fd must survive + } + expect(() => fs.fstatSync(probe.fd)).not.toThrow(); + }); + + it("async close does not close the unrelated fd", async () => { + using cwd = tempDir("dir-foreign-fd", { "probe.txt": "x" }); + using probe = openProbe(String(cwd)); + const d = new fs.Dir(probe.fd, String(cwd)); + await d.close().catch(() => {}); + expect(() => fs.fstatSync(probe.fd)).not.toThrow(); + }); + + it("Symbol.dispose does not close the unrelated fd", () => { + using cwd = tempDir("dir-foreign-fd", { "probe.txt": "x" }); + using probe = openProbe(String(cwd)); + { + using d = new fs.Dir(probe.fd, String(cwd)); + void d; + } + expect(() => fs.fstatSync(probe.fd)).not.toThrow(); + }); +});