Skip to content

zlib: prevent oversized ArrayBuffer retention in one-shot methods - #65647

Open
Abhirup0 wants to merge 1 commit into
nodejs:mainfrom
Abhirup0:fix/zlib-one-shot-memory-leak
Open

zlib: prevent oversized ArrayBuffer retention in one-shot methods#65647
Abhirup0 wants to merge 1 commit into
nodejs:mainfrom
Abhirup0:fix/zlib-one-shot-memory-leak

Conversation

@Abhirup0

Copy link
Copy Markdown

Fixes: #65600

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:

  1. Trims single-chunk one-shot results to exact size with Buffer.from() when
    the chunk is smaller than its backing ArrayBuffer.
  2. Releases internal references to _outBuffer, buffers, cb, and
    _writeState on stream completion, error, and close.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies. labels Aug 29, 2026
@MikeMcC399

Copy link
Copy Markdown
Contributor

This PR fails many tests.

Please make sure you have read and understood the following documents:

@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (7b0de5e) to head (d9ac576).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
lib/zlib.js 90.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65647      +/-   ##
==========================================
- Coverage   90.05%   90.05%   -0.01%     
==========================================
  Files         754      754              
  Lines      255722   255742      +20     
  Branches    48314    48317       +3     
==========================================
+ Hits       230281   230298      +17     
- Misses      16555    16565      +10     
+ Partials     8886     8879       -7     
Files with missing lines Coverage Δ
lib/zlib.js 97.99% <90.00%> (-0.15%) ⬇️

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

@Abhirup0
Abhirup0 force-pushed the fix/zlib-one-shot-memory-leak branch from d9ac576 to afc0e7a Compare August 30, 2026 07:51
@Abhirup0

Copy link
Copy Markdown
Author

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.

@Abhirup0
Abhirup0 force-pushed the fix/zlib-one-shot-memory-leak branch from afc0e7a to 6739f06 Compare August 30, 2026 08:19
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>
@Abhirup0
Abhirup0 force-pushed the fix/zlib-one-shot-memory-leak branch from 6739f06 to bb8cd2a Compare August 30, 2026 09:40
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. zlib Issues and PRs related to the zlib module and its compression dependencies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zlib: one-shot gzip()/deflate() results accumulate in arrayBuffers until OOM — GC never prompted (regression in v24.15.0)

4 participants