Revert "fs: fix close listener leak in FileHandle streams" by MikeMcC399 · Pull Request #65387 · nodejs/node

MikeMcC399 · 2026-08-18T15:57:32Z

This reverts commit 8488e13.

This is broken and should be reverted. Will unref the handle multiple times.

This reverts commit 8488e13. It was advised that the fix in question is broken since it will unref the handle multiple times. Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

github-actions · 2026-08-18T16:02:33Z

Fast-track has been requested by @ovflowd. Please 👍 to approve.

ovflowd · 2026-08-18T16:02:52Z

Removed need of CI, since we're reverting to previous code, this is known to be stable. Added fast-track.

ronag · 2026-08-18T16:08:18Z

Proper fix is to use eos/finished instead of 'close' listener

nodejs-github-bot · 2026-08-18T16:28:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75970/

panva · 2026-08-18T16:42:30Z

Removed need of CI, since we're reverting to previous code, this is known to be stable. Added fast-track.

The needs-ci label doesn't drive the need for CI. Removing it does absolutely nothing. CI is needed even for a revert, especially since other commits landed since that mean the codebase is not returning to a known state.

codecov · 2026-08-18T17:38:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.12%. Comparing base (cf30b2e) to head (5f3e792).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65387      +/-   ##
==========================================
+ Coverage   90.11%   90.12%   +0.01%     
==========================================
  Files         752      752              
  Lines      251861   251849      -12     
  Branches    47365    47363       -2     
==========================================
+ Hits       226955   226984      +29     
+ Misses      16238    16192      -46     
- Partials     8668     8673       +5

Files with missing lines	Coverage Δ
lib/internal/fs/streams.js	`93.10% <100.00%> (-0.14%)`	⬇️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-19T04:22:42Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75979/

nodejs-github-bot · 2026-08-19T06:42:37Z

Landed in 882a781

This reverts commit 8488e13. It was advised that the fix in question is broken since it will unref the handle multiple times. Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com> PR-URL: #65387 Refs: #64227 Refs: #64214 Reviewed-By: Claudio Wunder <cwunder@gnome.org> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

aduh95 · 2026-09-05T10:12:04Z

Please remember to add the dont-land labels to avoid the polluting the changelogs (too late for Node.js 26 atp, it already landed there)

MikeMcC399 · 2026-09-06T06:27:13Z

Please remember to add the dont-land labels to avoid the polluting the changelogs (too late for Node.js 26 atp, it already landed there)

This should probably be added as an instruction to the Collaborator guide > Reverting commits section.

Edit: PR #65848 proposed to document this, and has now landed to make the documentation clearer.

This reverts commit 8488e13. It was advised that the fix in question is broken since it will unref the handle multiple times. Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com> PR-URL: #65387 Refs: #64227 Refs: #64214 Reviewed-By: Claudio Wunder <cwunder@gnome.org> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

s/node/pull/64229 Signed-off-by: Rasad Regmi <rasad@workalaya.com> Title: fs: fix FileHandle close listener/ref leak in streams Description: `fileHandle.createReadStream()`/`createWriteStream()` with `autoClose: false` leaks a `'close'` listener (and an internal reference) on the `FileHandle` every time the stream finishes on its own, because `autoClose: false` also disables `autoDestroy`, so the stream never reaches `_destroy()` — the only place that previously released them. Enough such streams on one long-lived handle trips `MaxListenersExceededWarning`. `importFd()` in `lib/internal/fs/streams.js` now builds a single idempotent `releaseHandleRef` shared between `FileHandleOperations .close()` (the normal destroy path, unchanged for the default `autoClose: true` behavior) and a `finished()`-based safety net registered after the stream's readable/writable state is initialized, for streams that finish without ever calling `destroy()`. `finished()` correctly resolves on `'end'`/`'finish'` alone when the stream won't emit `'close'` (see `willEmitClose()` in `internal/streams/utils.js`), and the shared idempotency guard means it's safe even if a stream is also explicitly closed after finishing on its own — the exact scenario that caused a prior fix (nodejs#64227) to fs: fix FileHandle close listener/ref leak in streams createReadStream()/createWriteStream({ autoClose: false }) created from a FileHandle attached a 'close' listener and took a reference on the handle in importFd(), but only released either when the stream went through _destroy(). Since autoClose: false also disables autoDestroy, a stream that finishes on its own never reaches _destroy(), so the listener and reference leaked. Creating enough such streams on a long-lived handle (e.g. repeated ranged reads) eventually tripped MaxListenersExceededWarning. A prior fix (fixed in 64227, reverted in 65387) released the reference again on the stream's 'end'/'finish'/'error' independent of the destroy path, which could unref the handle twice if the stream was later also explicitly closed/destroyed - a normal thing to do after a stream naturally ends. This introduces a single idempotent release function shared by both paths (the normal destroy path and a finished()-based safety net for autoClose: false streams that never reach _destroy()), so the reference and listener are released exactly once regardless of how the stream ends. Fixes: nodejs#64214 Refs: nodejs#64227 Refs: nodejs#65387 Refs: nodejs#64229 Signed-off-by: Rasad Regmi <rasad@workalaya.com> Manual Git Steps (for you to run) cd "/home/rasadregmi/Desktop/Open Source Contributions Repos/node" git checkout -b fix-filehandle-stream-close-listener-leak main git add lib/internal/fs/streams.js test/parallel/test-fs-promises-file-hand git commit -s git push fork fix-filehandle-stream-close-listener-leak Commit Message fs: fix FileHandle close listener/ref leak in streams createReadStream()/createWriteStream({ autoClose: false }) created from a FileHandle attached a 'close' listener and took a reference on the handle in importFd(), but only released either when the stream went through _destroy(). Since autoClose: false also disables autoDestroy, a stream that finishes on its own never reaches _destroy(), so the listener and reference leaked. Creating enough such streams on a long-lived handle (e.g. repeated ranged reads) eventually tripped MaxListenersExceededWarning. A prior fix (fixed in 64227, reverted in 65387) released the reference again on the stream's 'end'/'finish'/'error' independent of the destroy path, which could unref the handle twice if the stream was later also explicitly closed/destroyed - a normal thing to do after a stream naturally ends. This introduces a single idempotent release function shared by both paths (the normal destroy path and a finished()-based safety net for autoClose: false streams that never reach _destroy()), so the reference and listener are released exactly once regardless of how the stream ends. Fixes: nodejs#64214 Refs: nodejs#64227 Refs: nodejs#65387 Refs: nodejs#64229 Signed-off-by: Rasad Regmi <regmirasad53@gmail.com>

createReadStream()/createWriteStream({ autoClose: false }) created from a FileHandle attached a 'close' listener and took a reference on the handle in importFd(), but only released either when the stream went through _destroy(). Since autoClose: false also disables autoDestroy, a stream that finishes on its own never reaches _destroy(), so the listener and reference leaked. Creating enough such streams on a long-lived handle (e.g. repeated ranged reads) eventually tripped MaxListenersExceededWarning. A prior fix (fixed in 64227, reverted in 65387) released the reference again on the stream's 'end'/'finish'/'error' independent of the destroy path, which could unref the handle twice if the stream was later also explicitly closed/destroyed - a normal thing to do after a stream naturally ends. This introduces a single idempotent release function shared by both paths (the normal destroy path and a finished()-based safety net for autoClose: false streams that never reach _destroy()), so the reference and listener are released exactly once regardless of how the stream ends. Fixes: nodejs#64214 Refs: nodejs#64227 Refs: nodejs#65387 Refs: nodejs#64229 Signed-off-by: Rasad Regmi <regmirasad53@gmail.com>

Revert "fs: fix close listener leak in FileHandle streams"

5f3e792

This reverts commit 8488e13. It was advised that the fix in question is broken since it will unref the handle multiple times. Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

nodejs-github-bot added fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Aug 18, 2026

MikeMcC399 marked this pull request as ready for review August 18, 2026 16:00

MikeMcC399 requested review from atlowChemi, ovflowd and ronag August 18, 2026 16:00

ovflowd approved these changes Aug 18, 2026

View reviewed changes

ovflowd added the fast-track PRs proposed for a shorter-than-standard waiting period before landing. label Aug 18, 2026

atlowChemi approved these changes Aug 18, 2026

View reviewed changes

ovflowd removed the needs-ci PRs that need a full CI run. label Aug 18, 2026

ronag approved these changes Aug 18, 2026

View reviewed changes

MikeMcC399 mentioned this pull request Aug 18, 2026

Close listener leak in fs/promises createReadStream #64214

Open

panva added needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. author ready PRs with CI started, the required approvals, and no outstanding review comments. labels Aug 18, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 18, 2026

lpinca approved these changes Aug 18, 2026

View reviewed changes

MikeMcC399 mentioned this pull request Aug 19, 2026

fs: fix close listener leak in FileHandle streams #64227

Merged

MikeMcC399 added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 19, 2026

nodejs-github-bot merged commit 882a781 into nodejs:main Aug 19, 2026
113 of 114 checks passed

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 19, 2026

MikeMcC399 deleted the revert-64227 branch August 19, 2026 06:46

Paraspandey-debugs mentioned this pull request Aug 20, 2026

test: add validation for listener removal in read streams #65421

Open

github-actions Bot mentioned this pull request Aug 25, 2026

2026-08-26, Version 26.8.0 (Current) #65551

Merged

aduh95 added dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels Sep 5, 2026

MikeMcC399 mentioned this pull request Sep 6, 2026

doc: expand revert commit collaborator instructions #65848

Merged

github-actions Bot mentioned this pull request Sep 7, 2026

2026-09-08, Version 24.21.0 'Krypton' (LTS) #65873

Merged

This comment was marked as resolved.

Sign in to view

rasadregmi mentioned this pull request Sep 17, 2026

fs: fix FileHandle close listener/ref leak in streams #66080

Open

Navigation Menu

Uh oh!

Revert "fs: fix close listener leak in FileHandle streams" - #65387

Revert "fs: fix close listener leak in FileHandle streams"#65387
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MikeMcC399:revert-64227

MikeMcC399 commented Aug 18, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

ovflowd commented Aug 18, 2026

Uh oh!

ronag commented Aug 18, 2026

Uh oh!

nodejs-github-bot commented Aug 18, 2026

Uh oh!

panva commented Aug 18, 2026

Uh oh!

codecov Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

nodejs-github-bot commented Aug 19, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 19, 2026

Uh oh!

aduh95 commented Sep 5, 2026 •

edited

Loading

Uh oh!

MikeMcC399 commented Sep 6, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

MikeMcC399 commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

ovflowd commented Aug 18, 2026

Uh oh!

ronag commented Aug 18, 2026

Uh oh!

nodejs-github-bot commented Aug 18, 2026

Uh oh!

panva commented Aug 18, 2026

Uh oh!

codecov Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Aug 19, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 19, 2026

Uh oh!

aduh95 commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcC399 commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

MikeMcC399 commented Aug 18, 2026 •

edited

Loading

codecov Bot commented Aug 18, 2026 •

edited

Loading

aduh95 commented Sep 5, 2026 •

edited

Loading

MikeMcC399 commented Sep 6, 2026 •

edited

Loading