From b53473f0e7eab68e4bc844474f286d834cbc6fc0 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 4 May 2016 21:18:21 +0200
Subject: [PATCH] zlib: remove `_closed` in source

This is purely cleanup and carries no visible behavioural changes.
Up to now, `this._closed` was used in zlib.js as a
synonym of `!this._handle`. This change makes this connection
explicit and removes the `_closed` property from zlib streams,
as the previous duplication has been the cause of subtle errors
like https://github.com/nodejs/node/issues/6034.

This also makes zlib errors lead to an explicit `_close()` call
rather than waiting for garbage collection to clean up the handle,
thus returning memory resources earlier in the case of an error.

Add a getter for `_closed` so that the property remains accessible
by legacy code.

PR-URL: https://github.com/nodejs/node/pull/6574
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/zlib.js                                  | 30 +++++++++++---------
 test/parallel/test-zlib-close-after-error.js |  2 ++
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/lib/zlib.js b/lib/zlib.js
index 3915e5ddabd318..6d9f47dca2266e 100644
--- a/lib/zlib.js
+++ b/lib/zlib.js
@@ -364,7 +364,7 @@ function Zlib(opts, mode) {
   this._handle.onerror = function(message, errno) {
     // there is no way to cleanly recover.
     // continuing only obscures problems.
-    self._handle = null;
+    _close(self);
     self._hadError = true;
 
     var error = new Error(message);
@@ -387,11 +387,16 @@ function Zlib(opts, mode) {
 
   this._buffer = Buffer.allocUnsafe(this._chunkSize);
   this._offset = 0;
-  this._closed = false;
   this._level = level;
   this._strategy = strategy;
 
   this.once('end', this.close);
+
+  Object.defineProperty(this, '_closed', {
+    get: () => { return !this._handle; },
+    configurable: true,
+    enumerable: true
+  });
 }
 
 util.inherits(Zlib, Transform);
@@ -412,7 +417,7 @@ Zlib.prototype.params = function(level, strategy, callback) {
   if (this._level !== level || this._strategy !== strategy) {
     var self = this;
     this.flush(binding.Z_SYNC_FLUSH, function() {
-      assert(!self._closed, 'zlib binding closed');
+      assert(self._handle, 'zlib binding closed');
       self._handle.params(level, strategy);
       if (!self._hadError) {
         self._level = level;
@@ -426,7 +431,7 @@ Zlib.prototype.params = function(level, strategy, callback) {
 };
 
 Zlib.prototype.reset = function() {
-  assert(!this._closed, 'zlib binding closed');
+  assert(this._handle, 'zlib binding closed');
   return this._handle.reset();
 };
 
@@ -469,15 +474,12 @@ function _close(engine, callback) {
   if (callback)
     process.nextTick(callback);
 
-  if (engine._closed)
+  // Caller may invoke .close after a zlib error (which will null _handle).
+  if (!engine._handle)
     return;
 
-  engine._closed = true;
-
-  // Caller may invoke .close after a zlib error (which will null _handle).
-  if (engine._handle) {
-    engine._handle.close();
-  }
+  engine._handle.close();
+  engine._handle = null;
 }
 
 function emitCloseNT(self) {
@@ -493,7 +495,7 @@ Zlib.prototype._transform = function(chunk, encoding, cb) {
   if (chunk !== null && !(chunk instanceof Buffer))
     return cb(new Error('invalid input'));
 
-  if (this._closed)
+  if (!this._handle)
     return cb(new Error('zlib binding closed'));
 
   // If it's the last chunk, or a final flush, we use the Z_FINISH flush flag
@@ -533,7 +535,7 @@ Zlib.prototype._processChunk = function(chunk, flushFlag, cb) {
       error = er;
     });
 
-    assert(!this._closed, 'zlib binding closed');
+    assert(this._handle, 'zlib binding closed');
     do {
       var res = this._handle.writeSync(flushFlag,
                                        chunk, // in
@@ -559,7 +561,7 @@ Zlib.prototype._processChunk = function(chunk, flushFlag, cb) {
     return buf;
   }
 
-  assert(!this._closed, 'zlib binding closed');
+  assert(this._handle, 'zlib binding closed');
   var req = this._handle.write(flushFlag,
                                chunk, // in
                                inOff, // in_off
diff --git a/test/parallel/test-zlib-close-after-error.js b/test/parallel/test-zlib-close-after-error.js
index 2570ca266a6754..8e21d159337c51 100644
--- a/test/parallel/test-zlib-close-after-error.js
+++ b/test/parallel/test-zlib-close-after-error.js
@@ -8,7 +8,9 @@ const zlib = require('zlib');
 const decompress = zlib.createGunzip(15);
 
 decompress.on('error', common.mustCall((err) => {
+  assert.strictEqual(decompress._closed, true);
   assert.doesNotThrow(() => decompress.close());
 }));
 
+assert.strictEqual(decompress._closed, false);
 decompress.write('something invalid');