zlib: prevent oversized ArrayBuffer retention in one-shot methods - #65647
zlib: prevent oversized ArrayBuffer retention in one-shot methods#65647Abhirup0 wants to merge 1 commit into
Conversation
|
This PR fails many tests. Please make sure you have read and understood the following documents: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65647 +/- ##
=======================================
Coverage 90.05% 90.06%
=======================================
Files 754 754
Lines 255722 255748 +26
Branches 48314 48331 +17
=======================================
+ Hits 230281 230327 +46
+ Misses 16555 16538 -17
+ Partials 8886 8883 -3
🚀 New features to boost your workflow:
|
d9ac576 to
afc0e7a
Compare
|
Updated the commit to follow commit message line wrapping and DCO sign-off guidelines, updated the test suite to use common.mustSucceed(), and narrowed the implementation cleanly to the single-chunk buffer trimming in zlibBufferOnEnd and processChunkSync. CI checks should now be green. |
afc0e7a to
6739f06
Compare
When one-shot convenience methods (zlib.gzip(), zlib.deflate(), zlib.brotliCompress(), zlib.zstdCompress(), etc.) complete with a single output chunk, zlibBufferOnEnd and processChunkSync returned bufs[0], which was a sub-slice of the 16 KB default _outBuffer. For small outputs, this retained the full 16,384-byte backing ArrayBuffer. Because this.close() synchronously frees the native C++ context and decrements the external memory accounter back to 0, V8 does not observe external memory pressure or significant JS heap growth. In long-running processes with comfortable heaps, dead ArrayBuffer allocations accumulated until the process ran out of memory. This change trims single-chunk one-shot results to exact size with Buffer.from() when the chunk is smaller than its backing ArrayBuffer. Fixes: nodejs#65600 Signed-off-by: Abhirup Karmakar <abhirup.karmakar00@gmail.com>
6739f06 to
bb8cd2a
Compare
|
You have forced-push several changes in the meantime, and CI continues to fail. Before you submit changes, you should test these locally. See https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#step-6-test |
|
Apologies for the noise and multiple CI runs, @MikeMcC399. The issue with the earlier test was an assumption about In Whenever convenient, could CI please be re-run on |
Fixes: #65600
When one-shot convenience methods (
zlib.gzip(),zlib.deflate(),zlib.brotliCompress(),zlib.zstdCompress(), etc.) complete with a singleoutput chunk,
zlibBufferOnEndandprocessChunkSyncreturnedbufs[0],which was a sub-slice of the 16 KB default
_outBuffer. For small outputs,this retained the full 16,384-byte backing
ArrayBuffer.Because
this.close()synchronously frees the native C++ context anddecrements the external memory accounter back to 0, V8 does not observe
external memory pressure or significant JS heap growth. In long-running
processes with comfortable heaps, dead
ArrayBufferallocations accumulateduntil the process ran out of memory.
This change:
Buffer.from()whenthe chunk is smaller than its backing
ArrayBuffer._outBuffer,buffers,cb, and_writeStateon stream completion, error, and close.