diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 554221ac614636..31c3c9e3a4fde5 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1068,6 +1068,7 @@ function setupHandle(socket, type, options) { if (typeof options.selectPadding === 'function') this[kSelectPadding] = options.selectPadding; handle.consume(socket._handle); + handle.ongracefulclosecomplete = this[kMaybeDestroy].bind(this, null); this[kHandle] = handle; if (this[kNativeFields]) { @@ -1589,6 +1590,10 @@ class Http2Session extends EventEmitter { if (typeof callback === 'function') this.once('close', callback); this.goaway(); + const handle = this[kHandle]; + if (handle) { + handle.setGracefulClose(); + } this[kMaybeDestroy](); } @@ -1609,11 +1614,13 @@ class Http2Session extends EventEmitter { // * session is closed and there are no more pending or open streams [kMaybeDestroy](error) { if (error == null) { + const handle = this[kHandle]; + const hasPendingData = !!handle && handle.hasPendingData(); const state = this[kState]; // Do not destroy if we're not closed and there are pending/open streams if (!this.closed || state.streams.size > 0 || - state.pendingStreams.size > 0) { + state.pendingStreams.size > 0 || hasPendingData) { return; } } @@ -3300,7 +3307,7 @@ function socketOnClose() { state.streams.forEach((stream) => stream.close(NGHTTP2_CANCEL)); state.pendingStreams.forEach((stream) => stream.close(NGHTTP2_CANCEL)); session.close(); - session[kMaybeDestroy](err); + closeSession(session, NGHTTP2_NO_ERROR, err); } } diff --git a/src/env_properties.h b/src/env_properties.h index 6ccc581034f4b2..3e82a9b543e9db 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -285,6 +285,7 @@ V(onsignal_string, "onsignal") \ V(onunpipe_string, "onunpipe") \ V(onwrite_string, "onwrite") \ + V(ongracefulclosecomplete_string, "ongracefulclosecomplete") \ V(openssl_error_stack, "opensslErrorStack") \ V(options_string, "options") \ V(order_string, "order") \ diff --git a/src/node_http2.cc b/src/node_http2.cc index b56b0d76cc7c0b..ea05aaacab8268 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -559,7 +559,8 @@ Http2Session::Http2Session(Http2State* http2_state, : AsyncWrap(http2_state->env(), wrap, AsyncWrap::PROVIDER_HTTP2SESSION), js_fields_(http2_state->env()->isolate()), session_type_(type), - http2_state_(http2_state) { + http2_state_(http2_state), + graceful_close_initiated_(false) { MakeWeak(); statistics_.session_type = type; statistics_.start_time = uv_hrtime(); @@ -765,6 +766,24 @@ void Http2Stream::EmitStatistics() { }); } +void Http2Session::HasPendingData(const FunctionCallbackInfo& args) { + Http2Session* session; + ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); + args.GetReturnValue().Set(session->HasPendingData()); +} + +bool Http2Session::HasPendingData() const { + nghttp2_session* session = session_.get(); + int want_write = nghttp2_session_want_write(session); + // It is expected that want_read will alway be 0 if graceful + // session close is initiated and goaway frame is sent. + int want_read = nghttp2_session_want_read(session); + if (want_write == 0 && want_read == 0) { + return false; + } + return true; +} + void Http2Session::EmitStatistics() { if (!HasHttp2Observer(env())) [[likely]] { return; @@ -1743,6 +1762,7 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) { void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) { Debug(this, "write finished with status %d", status); + MaybeNotifyGracefulCloseComplete(); CHECK(is_write_in_progress()); set_write_in_progress(false); @@ -1965,6 +1985,7 @@ uint8_t Http2Session::SendPendingData() { if (!res.async) { set_write_in_progress(false); ClearOutgoing(res.err); + MaybeNotifyGracefulCloseComplete(); } MaybeStopReading(); @@ -3478,6 +3499,8 @@ void Initialize(Local target, SetProtoMethod(isolate, session, "receive", Http2Session::Receive); SetProtoMethod(isolate, session, "destroy", Http2Session::Destroy); SetProtoMethod(isolate, session, "goaway", Http2Session::Goaway); + SetProtoMethod( + isolate, session, "hasPendingData", Http2Session::HasPendingData); SetProtoMethod(isolate, session, "settings", Http2Session::Settings); SetProtoMethod(isolate, session, "request", Http2Session::Request); SetProtoMethod( @@ -3498,6 +3521,8 @@ void Initialize(Local target, "remoteSettings", Http2Session::RefreshSettings); + SetProtoMethod( + isolate, session, "setGracefulClose", Http2Session::SetGracefulClose); SetConstructorFunction(context, target, "Http2Session", session); Local constants = Object::New(isolate); @@ -3552,6 +3577,38 @@ void Initialize(Local target, nghttp2_set_debug_vprintf_callback(NgHttp2Debug); #endif } + +void Http2Session::SetGracefulClose(const FunctionCallbackInfo& args) { + Http2Session* session; + ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); + CHECK_NOT_NULL(session); + // Set the graceful close flag + session->SetGracefulCloseInitiated(true); + + Debug(session, "Setting graceful close initiated flag"); +} + +void Http2Session::MaybeNotifyGracefulCloseComplete() { + nghttp2_session* session = session_.get(); + + if (!IsGracefulCloseInitiated()) { + return; + } + + int want_write = nghttp2_session_want_write(session); + int want_read = nghttp2_session_want_read(session); + bool should_notify = (want_write == 0 && want_read == 0); + + if (should_notify) { + Debug(this, "Notifying JS after write in graceful close mode"); + + // Make the callback to JavaScript + HandleScope scope(env()->isolate()); + MakeCallback(env()->ongracefulclosecomplete_string(), 0, nullptr); + } + + return; +} } // namespace http2 } // namespace node diff --git a/src/node_http2.h b/src/node_http2.h index 3ba05cbe7f9ce6..a60a7ba029db3e 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -712,6 +712,7 @@ class Http2Session : public AsyncWrap, static void Consume(const v8::FunctionCallbackInfo& args); static void Receive(const v8::FunctionCallbackInfo& args); static void Destroy(const v8::FunctionCallbackInfo& args); + static void HasPendingData(const v8::FunctionCallbackInfo& args); static void Settings(const v8::FunctionCallbackInfo& args); static void Request(const v8::FunctionCallbackInfo& args); static void SetNextStreamID(const v8::FunctionCallbackInfo& args); @@ -723,6 +724,7 @@ class Http2Session : public AsyncWrap, static void Ping(const v8::FunctionCallbackInfo& args); static void AltSvc(const v8::FunctionCallbackInfo& args); static void Origin(const v8::FunctionCallbackInfo& args); + static void SetGracefulClose(const v8::FunctionCallbackInfo& args); template static void RefreshSettings(const v8::FunctionCallbackInfo& args); @@ -735,6 +737,7 @@ class Http2Session : public AsyncWrap, BaseObjectPtr PopPing(); bool AddPing(const uint8_t* data, v8::Local callback); + bool HasPendingData() const; BaseObjectPtr PopSettings(); bool AddSettings(v8::Local callback); @@ -785,6 +788,13 @@ class Http2Session : public AsyncWrap, Statistics statistics_ = {}; + bool IsGracefulCloseInitiated() const { + return graceful_close_initiated_; + } + void SetGracefulCloseInitiated(bool value) { + graceful_close_initiated_ = value; + } + private: void EmitStatistics(); @@ -951,8 +961,13 @@ class Http2Session : public AsyncWrap, void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length); void ClearOutgoing(int status); + void MaybeNotifyGracefulCloseComplete(); + friend class Http2Scope; friend class Http2StreamListener; + + // Flag to indicate that JavaScript has initiated a graceful closure + bool graceful_close_initiated_ = false; }; struct Http2SessionPerformanceEntryTraits { diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js index bc0cb5ff619dc0..788253d29ae22f 100644 --- a/test/parallel/test-http2-client-rststream-before-connect.js +++ b/test/parallel/test-http2-client-rststream-before-connect.js @@ -5,16 +5,23 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); const h2 = require('http2'); +let client; const server = h2.createServer(); server.on('stream', (stream) => { - stream.on('close', common.mustCall()); - stream.respond(); - stream.end('ok'); + stream.on('close', common.mustCall(() => { + client.close(); + server.close(); + })); + stream.on('error', common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + name: 'Error', + message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR' + })); }); server.listen(0, common.mustCall(() => { - const client = h2.connect(`http://localhost:${server.address().port}`); + client = h2.connect(`http://localhost:${server.address().port}`); const req = client.request(); const closeCode = 1; @@ -52,8 +59,6 @@ server.listen(0, common.mustCall(() => { req.on('close', common.mustCall(() => { assert.strictEqual(req.destroyed, true); assert.strictEqual(req.rstCode, closeCode); - server.close(); - client.close(); })); req.on('error', common.expectsError({ diff --git a/test/parallel/test-http2-session-graceful-close.js b/test/parallel/test-http2-session-graceful-close.js new file mode 100644 index 00000000000000..174eb037dce5b4 --- /dev/null +++ b/test/parallel/test-http2-session-graceful-close.js @@ -0,0 +1,48 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const h2 = require('http2'); + +const server = h2.createServer(); +let session; + +server.on('session', common.mustCall(function(s) { + session = s; + session.on('close', common.mustCall(function() { + server.close(); + })); +})); + +server.listen(0, common.mustCall(function() { + const port = server.address().port; + + const url = `http://localhost:${port}`; + const client = h2.connect(url, common.mustCall(function() { + const headers = { + ':path': '/', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}` + }; + const request = client.request(headers); + request.on('response', common.mustCall(function(headers) { + assert.strictEqual(headers[':status'], 200); + }, 1)); + request.on('end', common.mustCall(function() { + client.close(); + })); + request.end(); + request.resume(); + })); + client.on('goaway', common.mustCallAtLeast(1)); +})); + +server.once('request', common.mustCall(function(request, response) { + response.on('finish', common.mustCall(function() { + session.close(); + })); + response.end(); +}));