Skip to content

Comments

repl: keep reference count for process.on('newListener')#61895

Open
addaleax wants to merge 1 commit intonodejs:mainfrom
addaleax:repl-newlistener-refcount
Open

repl: keep reference count for process.on('newListener')#61895
addaleax wants to merge 1 commit intonodejs:mainfrom
addaleax:repl-newlistener-refcount

Conversation

@addaleax
Copy link
Member

When investigating a memory leak in one of our applications, we discovered that this listener holds on to a REPLServer instance and all heap objects transitively kept alive by it by capturing as part of its closure.

It's cleaner to declare the listener outside of the REPLServer class and to actually clean it up properly when it is no longer required or meaningful, which is easily achieved through keeping a reference count.

When investigating a memory leak in one of our applications,
we discovered that this listener holds on to a `REPLServer`
instance and all heap objects transitively kept alive by it
by capturing as part of its closure.

It's cleaner to declare the listener outside of the `REPLServer`
class and to actually clean it up properly when it is no longer
required or meaningful, which is easily achieved through
keeping a reference count.
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels Feb 20, 2026
@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.76%. Comparing base (f632f3f) to head (86e6953).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61895      +/-   ##
==========================================
- Coverage   91.70%   89.76%   -1.94%     
==========================================
  Files         337      674     +337     
  Lines      140104   204899   +64795     
  Branches    21758    39381   +17623     
==========================================
+ Hits       128484   183935   +55451     
- Misses      11398    13237    +1839     
- Partials      222     7727    +7505     
Files with missing lines Coverage Δ
lib/repl.js 94.20% <100.00%> (+1.23%) ⬆️

... and 455 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.

@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 20, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 20, 2026
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants