diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-01-21 14:54:01 +0100 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2019-01-23 16:32:20 +0100 |
commit | 2b65399694440d0bab1c4e394898a4555e58c324 (patch) | |
tree | 8843314b2b38788a36b60b48454cee370927a0a2 | |
parent | d3806f9f3cded6bce9831f5d8ff88372ba7e5861 (diff) | |
download | android-node-v8-2b65399694440d0bab1c4e394898a4555e58c324.tar.gz android-node-v8-2b65399694440d0bab1c4e394898a4555e58c324.tar.bz2 android-node-v8-2b65399694440d0bab1c4e394898a4555e58c324.zip |
http2: allow fully synchronous `_final()`
HTTP/2 streams do not use the fact that the native
`StreamBase::Shutdown()` is asynchronous by default and
always finish synchronously.
Adding a status code for this scenario allows skipping an
expensive `MakeCallback()` C++/JS boundary crossing.
PR-URL: https://github.com/nodejs/node/pull/25609
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
-rw-r--r-- | lib/internal/http2/core.js | 4 | ||||
-rw-r--r-- | lib/net.js | 4 | ||||
-rw-r--r-- | src/node_http2.cc | 3 | ||||
-rw-r--r-- | src/stream_base.h | 10 |
4 files changed, 15 insertions, 6 deletions
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 400db84b56..fc99b50eae 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1821,7 +1821,9 @@ class Http2Stream extends Duplex { req.oncomplete = afterShutdown; req.callback = cb; req.handle = handle; - handle.shutdown(req); + const err = handle.shutdown(req); + if (err === 1) // synchronous finish + return afterShutdown.call(req, 0); } else { cb(); } diff --git a/lib/net.js b/lib/net.js index 7680c8860e..9eb7448c59 100644 --- a/lib/net.js +++ b/lib/net.js @@ -362,7 +362,9 @@ Socket.prototype._final = function(cb) { req.callback = cb; var err = this._handle.shutdown(req); - if (err) + if (err === 1) // synchronous finish + return afterShutdown.call(req, 0); + else if (err !== 0) return this.destroy(errnoException(err, 'shutdown')); }; diff --git a/src/node_http2.cc b/src/node_http2.cc index a3ce022835..ea60fb459c 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1948,8 +1948,7 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) { NGHTTP2_ERR_NOMEM); Debug(this, "writable side shutdown"); } - req_wrap->Done(0); - return 0; + return 1; } // Destroy the Http2Stream and render it unusable. Actual resources for the diff --git a/src/stream_base.h b/src/stream_base.h index 063c8714fd..cde3343d62 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -205,12 +205,16 @@ class StreamResource { // All of these methods may return an error code synchronously. // In that case, the finish callback should *not* be called. - // Perform a shutdown operation, and call req_wrap->Done() when finished. + // Perform a shutdown operation, and either call req_wrap->Done() when + // finished and return 0, return 1 for synchronous success, or + // a libuv error code for synchronous failures. virtual int DoShutdown(ShutdownWrap* req_wrap) = 0; // Try to write as much data as possible synchronously, and modify // `*bufs` and `*count` accordingly. This is a no-op by default. + // Return 0 for success and a libuv error code for failures. virtual int DoTryWrite(uv_buf_t** bufs, size_t* count); - // Perform a write of data, and call req_wrap->Done() when finished. + // Perform a write of data, and either call req_wrap->Done() when finished + // and return 0, or return a libuv error code for synchronous failures. virtual int DoWrite(WriteWrap* w, uv_buf_t* bufs, size_t count, @@ -274,6 +278,8 @@ class StreamBase : public StreamResource { // Shut down the current stream. This request can use an existing // ShutdownWrap object (that was created in JS), or a new one will be created. + // Returns 1 in case of a synchronous completion, 0 in case of asynchronous + // completion, and a libuv error case in case of synchronous failure. int Shutdown(v8::Local<v8::Object> req_wrap_obj = v8::Local<v8::Object>()); // Write data to the current stream. This request can use an existing |