From 8e542eaf5f4d8cf3355896cab2a603619ec2c43c Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 5 Sep 2018 17:52:21 +0200
Subject: [PATCH] zlib: fix memory leak for invalid input
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/nodejs/node/issues/22705

PR-URL: https://github.com/nodejs/node/pull/22713
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/node_zlib.cc                              |  6 ++--
 .../test-zlib-invalid-input-memory.js         | 28 +++++++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)
 create mode 100644 test/parallel/test-zlib-invalid-input-memory.js

diff --git a/src/node_zlib.cc b/src/node_zlib.cc
index cd15603f0b036d..3d7c6b004797f2 100644
--- a/src/node_zlib.cc
+++ b/src/node_zlib.cc
@@ -215,8 +215,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;
     }
 
@@ -364,6 +364,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;
 
@@ -388,7 +389,6 @@ class ZCtx : public AsyncWrap, public ThreadPoolWork {
                                            write_js_callback_);
     MakeCallback(cb, 0, nullptr);
 
-    Unref();
     if (pending_close_)
       Close();
   }
@@ -410,8 +410,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();
diff --git a/test/parallel/test-zlib-invalid-input-memory.js b/test/parallel/test-zlib-invalid-input-memory.js
new file mode 100644
index 00000000000000..d626e6e5b8df38
--- /dev/null
+++ b/test/parallel/test-zlib-invalid-input-memory.js
@@ -0,0 +1,28 @@
+// 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);
+    setImmediate(() => {
+      global.gc();
+      // Keep the event loop alive for seeing the async_hooks destroy hook
+      // we use for GC tracking...
+      // TODO(addaleax): This should maybe not be necessary?
+      setImmediate(() => {});
+    });
+  }));
+  onGC(strm, { ongc });
+}