Skip to content

Commit 6e5c208

Browse files
addaleaxBethGriggs
authored andcommitted
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 <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 29ed864 commit 6e5c208

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
@@ -1833,7 +1833,9 @@ class Http2Stream extends Duplex {
18331833
req.oncomplete = afterShutdown;
18341834
req.callback = cb;
18351835
req.handle = handle;
1836-
handle.shutdown(req);
1836+
const err = handle.shutdown(req);
1837+
if (err === 1) // synchronous finish
1838+
return afterShutdown.call(req, 0);
18371839
} else {
18381840
cb();
18391841
}

lib/net.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,9 @@ Socket.prototype._final = function(cb) {
357357
req.callback = cb;
358358
var err = this._handle.shutdown(req);
359359

360-
if (err)
360+
if (err === 1) // synchronous finish
361+
return afterShutdown.call(req, 0);
362+
else if (err !== 0)
361363
return this.destroy(errnoException(err, 'shutdown'));
362364
};
363365

src/node_http2.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -1956,8 +1956,7 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
19561956
NGHTTP2_ERR_NOMEM);
19571957
Debug(this, "writable side shutdown");
19581958
}
1959-
req_wrap->Done(0);
1960-
return 0;
1959+
return 1;
19611960
}
19621961

19631962
// 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,
@@ -272,6 +276,8 @@ class StreamBase : public StreamResource {
272276

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

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

0 commit comments

Comments
 (0)