Skip to content

zlib: fix memory leak for invalid input #22713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
addaleax wants to merge 3 commits into from
Closed

zlib: fix memory leak for invalid input #22713

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2be5eb6
zlib: fix memory leak for invalid input
addaleax Sep 5, 2018
705fb2b
fixup! zlib: fix memory leak for invalid input
addaleax Sep 5, 2018
68d9798
fixup! zlib: fix memory leak for invalid input
addaleax Sep 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
zlib: fix memory leak for invalid input 
Don’t toggle the weak/strong reference flag from the error
handler, that’s too confusing. Instead, always do it in the
code that handles the write call.

Fixes: #22705
  • Loading branch information
@addaleax
addaleax committed Sep 5, 2018
commit 2be5eb64cea05f5e7cc72c913c62e79cce5ff0ef
6 changes: 2 additions & 4 deletions src/node_zlib.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
ctx->write_result_[0] = ctx->strm_.avail_out;
ctx->write_result_[1] = ctx->strm_.avail_in;
ctx->write_in_progress_ = false;
ctx->Unref();
}
ctx->Unref();
return;
}

Expand Down Expand Up @@ -363,6 +363,7 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
// v8 land!
void AfterThreadPoolWork(int status) override {
AllocScope alloc_scope(this);
OnScopeLeave on_scope_leave([&]() { Unref(); });

write_in_progress_ = false;

Expand All @@ -387,7 +388,6 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
write_js_callback_);
MakeCallback(cb, 0, nullptr);

Unref();
if (pending_close_)
Close();
}
Expand All @@ -409,8 +409,6 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
MakeCallback(env()->onerror_string(), arraysize(args), args);

// no hope of rescue.
if (write_in_progress_)
Unref();
write_in_progress_ = false;
if (pending_close_)
Close();
Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-zlib-invalid-input-memory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Flags: --expose-gc
'use strict';
const common = require('../common');
const onGC = require('../common/ongc');
const assert = require('assert');
const zlib = require('zlib');

// Checks that, if a zlib context fails with an error, it can still be GC'ed:
// Refs: https://github.com/nodejs/node/issues/22705

const ongc = common.mustCall();

{
const input = Buffer.from('foobar');
const strm = zlib.createInflate();
strm.end(input);
strm.once('error', common.mustCall((err) => assert(err)));
onGC(strm, { ongc });
}

setImmediate(() => global.gc());