Skip to content

util: fix debounce/throttle abort listener leak - #66839

Open
vedchaudhari wants to merge 1 commit into
nodejs:mainfrom
vedchaudhari:util-debounce-signal-leak
Open

vedchaudhari wants to merge 1 commit into
nodejs:mainfrom
vedchaudhari:util-debounce-signal-leak

Conversation

@vedchaudhari

@vedchaudhari vedchaudhari commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

util.debounce() and util.throttle() add an abort listener to options.signal when they're created and never remove it, so a long-lived signal keeps every function created with it in memory:

const ac = new AbortController();
for (let i = 0; i < 5; i++) util.debounce(() => {}, 100, { signal: ac.signal });
events.getEventListeners(ac.signal, 'abort').length;
// before: 5
// after:  0

This only keeps the listener while calls are pending, and removes it with the Symbol.dispose method that addAbortListener() returns. Calls after an abort still reject, since every call already checks signal.aborted.

All functions share one long-lived AbortSignal:

Debounced functions created Heap after GC on main Heap after GC with this PR
10,000 29 MB 4 MB
30,000 78 MB 4 MB
50,000 127 MB 4 MB

Normal calls take the same time as before. Creating a debounced function with a signal is about 20% faster, and abort() with 5,000 idle functions takes 0.2 ms instead of 9.4 ms. The only extra cost is under 1 µs when a call starts waiting, to add and remove the listener.

Performance numbers

Median of 9 runs, old and new code on the same build, Windows x64:

Scenario main This PR
Calls within one debounce window 110 ns 115 ns
Throttle calls that run immediately 368 ns 356 ns
Creating a debounced function with a signal 5.1 µs 4.1 µs
abort() with 5,000 idle functions 9.4 ms 0.2 ms
Worst case: debounce call + flush() every time, with a signal 457 ns 1,147 ns
Worst case: throttle queues one call every time, with a signal 416 ns 1,049 ns

Added tests; they fail on main and pass with this change.

Refs: #65899

Assisted-by: Claude Code. I reproduced the leak on main, reviewed every change, and ran the debounce/throttle tests and lint myself.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Oct 18, 2026
@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.49%. Comparing base (f88a1d2) to head (9d4466e).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #66839    +/-   ##
========================================
  Coverage   90.49%   90.49%            
========================================
  Files         797      797            
  Lines      277780   278249   +469     
  Branches    53360    53465   +105     
========================================
+ Hits       251388   251813   +425     
- Misses      16791    16806    +15     
- Partials     9601     9630    +29     
Files with missing lines Coverage Δ
lib/internal/util/debounce.js 97.61% <100.00%> (+0.15%) ⬆️
lib/internal/util/throttle.js 94.27% <100.00%> (+0.21%) ⬆️

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

@bakkot

bakkot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

The addAbortListener helper returns an object with a Symbol.dispose method that removes the listener; might be simpler to just use that?

debounce() and throttle() added an abort listener to the signal when
they were created and never removed it, so a long-lived signal kept
every debounced or throttled function, and its data, in memory.

Only listen for abort while calls are pending. Calls made after the
signal is aborted still reject, since each call checks signal.aborted.

Assisted-by: a closed-source coding agent
Signed-off-by: vedchaudhari <vedc2853@gmail.com>
@vedchaudhari
vedchaudhari force-pushed the util-debounce-signal-leak branch from 7b08eab to 9d4466e Compare October 19, 2026 18:22
@vedchaudhari

Copy link
Copy Markdown
Contributor Author

The addAbortListener helper returns an object with a Symbol.dispose method that removes the listener; might be simpler to just use that?

Thanks, good point. Switched to addAbortListener() and its Symbol.dispose. I had used addEventListener and removeEventListener directly for a small speedup but it is not worth the extra code. Also updated the numbers in the description.

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

Labels

needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants