From 5cd6cccf445ace23e8c2d51ade3be48dbaa5e463 Mon Sep 17 00:00:00 2001
From: zero1five <zerodengyin@gmail.com>
Date: Sun, 2 Jun 2019 03:04:56 +0800
Subject: [PATCH 1/2] stream: add writableFinished

add a new getter to duplex stream to replace the property `this
.writableState.finished` of the object that inherited duplex.

Refs: https://github.com/nodejs/node/issues/445
---
 doc/api/stream.md                             | 10 +++++++
 lib/_stream_duplex.js                         | 10 +++++++
 lib/_stream_writable.js                       | 10 +++++++
 .../test-stream-duplex-writable-finished.js   | 30 +++++++++++++++++++
 .../parallel/test-stream-writable-finished.js | 30 +++++++++++++++++++
 5 files changed, 90 insertions(+)
 create mode 100644 test/parallel/test-stream-duplex-writable-finished.js
 create mode 100644 test/parallel/test-stream-writable-finished.js

diff --git a/doc/api/stream.md b/doc/api/stream.md
index abefb4aa6bba77..06e63798f5b817 100644
--- a/doc/api/stream.md
+++ b/doc/api/stream.md
@@ -502,6 +502,16 @@ This property contains the number of bytes (or objects) in the queue
 ready to be written. The value provides introspection data regarding
 the status of the `highWaterMark`.
 
+##### writable.writableFinished
+<!-- YAML
+added: v12.4.0
+-->
+
+* {boolean}
+
+Is `true` if all data has been flushed to the underlying system. After
+the [`'finish'`][] event has been emitted.
+
 ##### writable.writableObjectMode
 <!-- YAML
 added: v12.3.0
diff --git a/lib/_stream_duplex.js b/lib/_stream_duplex.js
index 49520c4074e5a2..d20877e921e445 100644
--- a/lib/_stream_duplex.js
+++ b/lib/_stream_duplex.js
@@ -98,6 +98,16 @@ Object.defineProperty(Duplex.prototype, 'writableLength', {
   }
 });
 
+Object.defineProperty(Duplex.prototype, 'writableFinished', {
+  // Making it explicit this property is not enumerable
+  // because otherwise some prototype manipulation in
+  // userland will fail
+  enumerable: false,
+  get() {
+    return this._writableState.finished;
+  }
+});
+
 // The no-half-open enforcer
 function onend() {
   // If the writable side ended, then we're ok.
diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js
index 267418e9d7c6df..e85b4dead859bc 100644
--- a/lib/_stream_writable.js
+++ b/lib/_stream_writable.js
@@ -714,6 +714,16 @@ Object.defineProperty(Writable.prototype, 'writableObjectMode', {
   }
 });
 
+Object.defineProperty(Writable.prototype, 'writableFinished', {
+  // Making it explicit this property is not enumerable
+  // because otherwise some prototype manipulation in
+  // userland will fail
+  enumerable: false,
+  get() {
+    return this._writableState.finished;
+  }
+});
+
 Writable.prototype.destroy = destroyImpl.destroy;
 Writable.prototype._undestroy = destroyImpl.undestroy;
 Writable.prototype._destroy = function(err, cb) {
diff --git a/test/parallel/test-stream-duplex-writable-finished.js b/test/parallel/test-stream-duplex-writable-finished.js
new file mode 100644
index 00000000000000..6d9e860b6150d5
--- /dev/null
+++ b/test/parallel/test-stream-duplex-writable-finished.js
@@ -0,0 +1,30 @@
+'use strict';
+
+const common = require('../common');
+const { Duplex } = require('stream');
+const assert = require('assert');
+
+// basic
+{
+  // Find it on Duplex.prototype
+  assert(Duplex.prototype.hasOwnProperty('writableFinished'));
+}
+
+// event
+{
+  const duplex = new Duplex();
+
+  duplex._write = (chunk, encoding, cb) => {
+    // The state finished should start in false.
+    assert.strictEqual(duplex.writableFinished, false);
+    cb();
+  };
+
+  duplex.on('finish', common.mustCall(() => {
+    assert.strictEqual(duplex.writableFinished, true);
+  }));
+
+  duplex.end('testing finished state', common.mustCall(() => {
+    assert.strictEqual(duplex.writableFinished, true);
+  }));
+}
diff --git a/test/parallel/test-stream-writable-finished.js b/test/parallel/test-stream-writable-finished.js
new file mode 100644
index 00000000000000..a5dfc060256a02
--- /dev/null
+++ b/test/parallel/test-stream-writable-finished.js
@@ -0,0 +1,30 @@
+'use strict';
+
+const common = require('../common');
+const { Writable } = require('stream');
+const assert = require('assert');
+
+// basic
+{
+  // Find it on Writable.prototype
+  assert(Writable.prototype.hasOwnProperty('writableFinished'));
+}
+
+// event
+{
+  const writable = new Writable();
+
+  writable._write = (chunk, encoding, cb) => {
+    // The state finished should start in false.
+    assert.strictEqual(writable.writableFinished, false);
+    cb();
+  };
+
+  writable.on('finish', common.mustCall(() => {
+    assert.strictEqual(writable.writableFinished, true);
+  }));
+
+  writable.end('testing finished state', common.mustCall(() => {
+    assert.strictEqual(writable.writableFinished, true);
+  }));
+}

From 2c66f6c09bcf79c8dfe955144be4fda5bbabca8b Mon Sep 17 00:00:00 2001
From: zero1five <zerodengyin@gmail.com>
Date: Sun, 2 Jun 2019 19:39:03 +0800
Subject: [PATCH 2/2] http2: use writableFinished instead of _writableState

---
 lib/internal/http2/core.js | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 8c05ff626792d7..63f9da5564d6d8 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -1556,7 +1556,7 @@ function closeStream(stream, code, rstStreamStatus = kSubmitRstStream) {
   stream.setTimeout(0);
   stream.removeAllListeners('timeout');
 
-  const { ending, finished } = stream._writableState;
+  const { ending } = stream._writableState;
 
   if (!ending) {
     // If the writable side of the Http2Stream is still open, emit the
@@ -1572,7 +1572,7 @@ function closeStream(stream, code, rstStreamStatus = kSubmitRstStream) {
 
   if (rstStreamStatus !== kNoRstStream) {
     const finishFn = finishCloseStream.bind(stream, code);
-    if (!ending || finished || code !== NGHTTP2_NO_ERROR ||
+    if (!ending || stream.writableFinished || code !== NGHTTP2_NO_ERROR ||
         rstStreamStatus === kForceRstStream)
       finishFn();
     else
@@ -1982,8 +1982,7 @@ class Http2Stream extends Duplex {
       return;
     }
 
-    // TODO(mcollina): remove usage of _*State properties
-    if (this._writableState.finished) {
+    if (this.writableFinished) {
       if (!this.readable && this.closed) {
         this.destroy();
         return;