Skip to content

Commit 9a410a1

Browse files
committed
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: #25609 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 3c661f0 commit 9a410a1

File tree

4 files changed

+15
-6
lines changed

4 files changed

+15
-6
lines changed

lib/internal/http2/core.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -1821,7 +1821,9 @@ class Http2Stream extends Duplex {
18211821
req.oncomplete = afterShutdown;
18221822
req.callback = cb;
18231823
req.handle = handle;
1824-
handle.shutdown(req);
1824+
const err = handle.shutdown(req);
1825+
if (err === 1) // synchronous finish
1826+
return afterShutdown.call(req, 0);
18251827
} else {
18261828
cb();
18271829
}

lib/net.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,9 @@ Socket.prototype._final = function(cb) {
362362
req.callback = cb;
363363
var err = this._handle.shutdown(req);
364364

365-
if (err)
365+
if (err === 1) // synchronous finish
366+
return afterShutdown.call(req, 0);
367+
else if (err !== 0)
366368
return this.destroy(errnoException(err, 'shutdown'));
367369
};
368370

src/node_http2.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -1948,8 +1948,7 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
19481948
NGHTTP2_ERR_NOMEM);
19491949
Debug(this, "writable side shutdown");
19501950
}
1951-
req_wrap->Done(0);
1952-
return 0;
1951+
return 1;
19531952
}
19541953

19551954
// Destroy the Http2Stream and render it unusable. Actual resources for the

src/stream_base.h

+8-2
Original file line numberDiff line numberDiff line change
@@ -205,12 +205,16 @@ class StreamResource {
205205
// All of these methods may return an error code synchronously.
206206
// In that case, the finish callback should *not* be called.
207207

208-
// Perform a shutdown operation, and call req_wrap->Done() when finished.
208+
// Perform a shutdown operation, and either call req_wrap->Done() when
209+
// finished and return 0, return 1 for synchronous success, or
210+
// a libuv error code for synchronous failures.
209211
virtual int DoShutdown(ShutdownWrap* req_wrap) = 0;
210212
// Try to write as much data as possible synchronously, and modify
211213
// `*bufs` and `*count` accordingly. This is a no-op by default.
214+
// Return 0 for success and a libuv error code for failures.
212215
virtual int DoTryWrite(uv_buf_t** bufs, size_t* count);
213-
// Perform a write of data, and call req_wrap->Done() when finished.
216+
// Perform a write of data, and either call req_wrap->Done() when finished
217+
// and return 0, or return a libuv error code for synchronous failures.
214218
virtual int DoWrite(WriteWrap* w,
215219
uv_buf_t* bufs,
216220
size_t count,
@@ -274,6 +278,8 @@ class StreamBase : public StreamResource {
274278

275279
// Shut down the current stream. This request can use an existing
276280
// ShutdownWrap object (that was created in JS), or a new one will be created.
281+
// Returns 1 in case of a synchronous completion, 0 in case of asynchronous
282+
// completion, and a libuv error case in case of synchronous failure.
277283
int Shutdown(v8::Local<v8::Object> req_wrap_obj = v8::Local<v8::Object>());
278284

279285
// Write data to the current stream. This request can use an existing

0 commit comments

Comments
 (0)