nodejs
lib: clean up persisted signals when they are settled
Refs #55328 Fixes #55328
This PR adds a finalizer to the non-composed signals used in AbortSignal.any. With that finalizer, those settled signals can be removed from gcPersistentSignals.
Comparison
This comparison shows that settled signals are being collected.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88.00%. Comparing base (
3bb1d28) to head (c11785e). Report is 1415 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #56001 +/- ##
==========================================
+ Coverage 87.95% 88.00% +0.04%
==========================================
Files 656 656
Lines 188310 189010 +700
Branches 35963 35993 +30
==========================================
+ Hits 165630 166330 +700
+ Misses 15854 15839 -15
- Partials 6826 6841 +15
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/internal/abort_controller.js | 98.13% <100.00%> (+0.07%) |
:arrow_up: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Not blocking, but looks like this fixed the repro reported by the OP, but not the one mentioned at #55328 (comment)
Oh, that's the code I used to reproduce locally and ensure the memory stops growing infinitely.
The memory is still pretty high - I think it's because it has those orphan listeners for a while - but it reaches a peak and stops growing forever.
The added test seems very flaky. For example https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel9-s390x/47103/testReport/junit/(root)/parallel/test_abortsignal_drop_settled_signals/
The added test seems very flaky. For example https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel9-s390x/47103/testReport/junit/(root)/parallel/test_abortsignal_drop_settled_signals/
It is actually. https://github.com/nodejs/node/issues/56190
Should we revert instead of fixing it?
You're right. In this example the failure is in drops settled dependant signals when signal is composite.
We should probably revert if we don't find a fix quickly
You're right. In this example the failure is in
drops settled dependant signals when signal is composite. We should probably revert if we don't find a fix quickly
How much time do I have? I'm investigating it right now.
You're right. In this example the failure is in
drops settled dependant signals when signal is composite. We should probably revert if we don't find a fix quicklyHow much time do I have? I'm investigating it right now.
I moved this new test to a single file to examine it better.
I'm running tools/test.py --repeat 9999 test/parallel/test-abortsignal-with-listeners.js at this moment