fs: do not close FileHandle on stream destroy with autoClose: false - #66669

Fixes: #45721
Refs: #45909, #45917

Problem

const fh = await fs.promises.open('file.txt');
const stream = fh.createReadStream({ autoClose: false });
for await (const chunk of stream) break; // destroys the stream
// fh is now closed (fh.fd === -1), even though autoClose is false

Root cause

Each FileHandle keeps a reference count (kRefs), which starts at 1 for the owner. A stream created from the handle takes its own reference in importFd(). When the stream is destroyed, _destroy() always calls close(), which ends up in FileHandleOperations.close(). That function did two things:

1. handle[kUnref](), which releases the stream's reference. This part is correct.
2. handle.close(), which releases the owner's reference and closes the fd.

Step 2 runs whatever autoClose is set to. autoClose only maps to the stream's autoDestroy, so it controls whether the stream destroys itself on 'end'/'error'. It has no effect on what destroying the stream does. Breaking out of for await, stream.destroy(), stream.close() and pipeline() teardown all destroy the stream, so all of them closed a handle the user said they own. The docs for filehandle.createReadStream() and filehandle.createWriteStream() say that with autoClose: false "the file descriptor won't be closed … It is the application's responsibility to close it". Newer FileHandle APIs already work that way (readableWebStream({ autoClose }), writer({ autoClose }), pull({ autoClose })).

There was also a second leak. importFd() added a 'close' listener on the FileHandle for every stream and never removed it. A long-lived handle that serves many streams therefore kept all of them alive and eventually hit MaxListenersExceededWarning. The issue reporter mentions this case.

Fix

In FileHandleOperations.close() (shared by ReadStream and WriteStream):

• always remove the stream's 'close' listener from the handle;
• always release the stream's own reference;
• call handle.close() only when stream.autoClose is true. Otherwise, call back on the next tick.

Streams opened from a raw numeric fd or a path are unchanged. So is the default autoClose: true behaviour, and so is closing the FileHandle while a stream is still using it: the handle's 'close' event still destroys the stream, and releasing the stream's reference still finishes the pending close.

Why this differs from #45909

#45909 (ronag) had the same goal. It decided whether to close by checking handle[kFd] !== -1 after the unref ("someone else has a ref"). The owner always holds a ref, so that condition is also true with autoClose: true. The handle would then never be closed, which breaks the documented default and existing tests ("LGTM except the test failures"). This PR keys on the option the user actually passed (autoClose) instead. It also leaves out #45909's unrelated change that moved FileHandle's 'close' emit after the native close, because that changes event timing for every FileHandle user. The listener cleanup from #45909 is kept. #45917 was closed by its author in favour of #45909.

Tests

New test/parallel/test-fs-stream-file-handle-autoclose-false.js:

• for await … break on a ReadStream 15 times on one handle: the handle stays open, the 'close' listener count stays flat, and no MaxListenersExceededWarning is emitted;
• explicit destroy() on a ReadStream and a WriteStream with autoClose: false leaves the handle usable;
• closing the FileHandle while an autoClose: false stream holds a ref still closes both;
• autoClose: true still closes the handle on destroy and removes the listener.

Existing test/parallel/test-fs-*stream* and test-filehandle* tests pass. [fill in actual results]

Semver

I consider this a bug fix (semver-patch): it makes the implementation match the documented behaviour of autoClose: false for FileHandle streams. Code that relied on destroy() closing a FileHandle it created with autoClose: false would now have to call filehandle.close() itself, which the docs already require. Unclosed FileHandles are still closed on GC, with a warning. If reviewers think this should be semver-major, I'm fine with that label.

AI-assistance disclosure: I used an AI coding assistant to help investigate the issue and draft the code, tests and this description. I reviewed, tested and understand all of the changes.