GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — http2: fix graceful session close · nodejs/node@a365564

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • lib/internal/http2
    • core.js
  • src
    • env_properties.h
    • node_http2.cc
    • node_http2.h
  • test/parallel
    • test-http2-client-rststream-before-connect.js
    • test-http2-session-graceful-close.js
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+9-2Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,7 @@ function setupHandle(socket, type, options) {
10681068
if (typeof options.selectPadding === 'function')
10691069
this[kSelectPadding] = options.selectPadding;
10701070
handle.consume(socket._handle);
1071+
handle.ongracefulclosecomplete = this[kMaybeDestroy].bind(this, null);
10711072

10721073
this[kHandle] = handle;
10731074
if (this[kNativeFields]) {
@@ -1589,6 +1590,10 @@ class Http2Session extends EventEmitter {
15891590
if (typeof callback === 'function')
15901591
this.once('close', callback);
15911592
this.goaway();
1593+
const handle = this[kHandle];
1594+
if (handle) {
1595+
handle.setGracefulClose();
1596+
}
15921597
this[kMaybeDestroy]();
15931598
}
15941599

@@ -1609,11 +1614,13 @@ class Http2Session extends EventEmitter {
16091614
// * session is closed and there are no more pending or open streams
16101615
[kMaybeDestroy](error) {
16111616
if (error == null) {
1617+
const handle = this[kHandle];
1618+
const hasPendingData = !!handle && handle.hasPendingData();
16121619
const state = this[kState];
16131620
// Do not destroy if we're not closed and there are pending/open streams
16141621
if (!this.closed ||
16151622
state.streams.size > 0 ||
1616-
state.pendingStreams.size > 0) {
1623+
state.pendingStreams.size > 0 || hasPendingData) {
16171624
return;
16181625
}
16191626
}
@@ -3300,7 +3307,7 @@ function socketOnClose() {
33003307
state.streams.forEach((stream) => stream.close(NGHTTP2_CANCEL));
33013308
state.pendingStreams.forEach((stream) => stream.close(NGHTTP2_CANCEL));
33023309
session.close();
3303-
session[kMaybeDestroy](err);
3310+
closeSession(session, NGHTTP2_NO_ERROR, err);
33043311
}
33053312
}
33063313

Collapse file

‎src/env_properties.h‎

Copy file name to clipboardExpand all lines: src/env_properties.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@
285285
V(onsignal_string, "onsignal") \
286286
V(onunpipe_string, "onunpipe") \
287287
V(onwrite_string, "onwrite") \
288+
V(ongracefulclosecomplete_string, "ongracefulclosecomplete") \
288289
V(openssl_error_stack, "opensslErrorStack") \
289290
V(options_string, "options") \
290291
V(order_string, "order") \
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+58-1Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,8 @@ Http2Session::Http2Session(Http2State* http2_state,
559559
: AsyncWrap(http2_state->env(), wrap, AsyncWrap::PROVIDER_HTTP2SESSION),
560560
js_fields_(http2_state->env()->isolate()),
561561
session_type_(type),
562-
http2_state_(http2_state) {
562+
http2_state_(http2_state),
563+
graceful_close_initiated_(false) {
563564
MakeWeak();
564565
statistics_.session_type = type;
565566
statistics_.start_time = uv_hrtime();
@@ -765,6 +766,24 @@ void Http2Stream::EmitStatistics() {
765766
});
766767
}
767768

769+
void Http2Session::HasPendingData(const FunctionCallbackInfo<Value>& args) {
770+
Http2Session* session;
771+
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
772+
args.GetReturnValue().Set(session->HasPendingData());
773+
}
774+
775+
bool Http2Session::HasPendingData() const {
776+
nghttp2_session* session = session_.get();
777+
int want_write = nghttp2_session_want_write(session);
778+
// It is expected that want_read will alway be 0 if graceful
779+
// session close is initiated and goaway frame is sent.
780+
int want_read = nghttp2_session_want_read(session);
781+
if (want_write == 0 && want_read == 0) {
782+
return false;
783+
}
784+
return true;
785+
}
786+
768787
void Http2Session::EmitStatistics() {
769788
if (!HasHttp2Observer(env())) [[likely]] {
770789
return;
@@ -1743,6 +1762,7 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
17431762
void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
17441763
Debug(this, "write finished with status %d", status);
17451764

1765+
MaybeNotifyGracefulCloseComplete();
17461766
CHECK(is_write_in_progress());
17471767
set_write_in_progress(false);
17481768

@@ -1965,6 +1985,7 @@ uint8_t Http2Session::SendPendingData() {
19651985
if (!res.async) {
19661986
set_write_in_progress(false);
19671987
ClearOutgoing(res.err);
1988+
MaybeNotifyGracefulCloseComplete();
19681989
}
19691990

19701991
MaybeStopReading();
@@ -3476,6 +3497,8 @@ void Initialize(Local<Object> target,
34763497
SetProtoMethod(isolate, session, "receive", Http2Session::Receive);
34773498
SetProtoMethod(isolate, session, "destroy", Http2Session::Destroy);
34783499
SetProtoMethod(isolate, session, "goaway", Http2Session::Goaway);
3500+
SetProtoMethod(
3501+
isolate, session, "hasPendingData", Http2Session::HasPendingData);
34793502
SetProtoMethod(isolate, session, "settings", Http2Session::Settings);
34803503
SetProtoMethod(isolate, session, "request", Http2Session::Request);
34813504
SetProtoMethod(
@@ -3496,6 +3519,8 @@ void Initialize(Local<Object> target,
34963519
"remoteSettings",
34973520
Http2Session::RefreshSettings<nghttp2_session_get_remote_settings,
34983521
false>);
3522+
SetProtoMethod(
3523+
isolate, session, "setGracefulClose", Http2Session::SetGracefulClose);
34993524
SetConstructorFunction(context, target, "Http2Session", session);
35003525

35013526
Local<Object> constants = Object::New(isolate);
@@ -3550,6 +3575,38 @@ void Initialize(Local<Object> target,
35503575
nghttp2_set_debug_vprintf_callback(NgHttp2Debug);
35513576
#endif
35523577
}
3578+
3579+
void Http2Session::SetGracefulClose(const FunctionCallbackInfo<Value>& args) {
3580+
Http2Session* session;
3581+
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
3582+
CHECK_NOT_NULL(session);
3583+
// Set the graceful close flag
3584+
session->SetGracefulCloseInitiated(true);
3585+
3586+
Debug(session, "Setting graceful close initiated flag");
3587+
}
3588+
3589+
void Http2Session::MaybeNotifyGracefulCloseComplete() {
3590+
nghttp2_session* session = session_.get();
3591+
3592+
if (!IsGracefulCloseInitiated()) {
3593+
return;
3594+
}
3595+
3596+
int want_write = nghttp2_session_want_write(session);
3597+
int want_read = nghttp2_session_want_read(session);
3598+
bool should_notify = (want_write == 0 && want_read == 0);
3599+
3600+
if (should_notify) {
3601+
Debug(this, "Notifying JS after write in graceful close mode");
3602+
3603+
// Make the callback to JavaScript
3604+
HandleScope scope(env()->isolate());
3605+
MakeCallback(env()->ongracefulclosecomplete_string(), 0, nullptr);
3606+
}
3607+
3608+
return;
3609+
}
35533610
} // namespace http2
35543611
} // namespace node
35553612

Collapse file

‎src/node_http2.h‎

Copy file name to clipboardExpand all lines: src/node_http2.h
+15Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,7 @@ class Http2Session : public AsyncWrap,
712712
static void Consume(const v8::FunctionCallbackInfo<v8::Value>& args);
713713
static void Receive(const v8::FunctionCallbackInfo<v8::Value>& args);
714714
static void Destroy(const v8::FunctionCallbackInfo<v8::Value>& args);
715+
static void HasPendingData(const v8::FunctionCallbackInfo<v8::Value>& args);
715716
static void Settings(const v8::FunctionCallbackInfo<v8::Value>& args);
716717
static void Request(const v8::FunctionCallbackInfo<v8::Value>& args);
717718
static void SetNextStreamID(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -723,6 +724,7 @@ class Http2Session : public AsyncWrap,
723724
static void Ping(const v8::FunctionCallbackInfo<v8::Value>& args);
724725
static void AltSvc(const v8::FunctionCallbackInfo<v8::Value>& args);
725726
static void Origin(const v8::FunctionCallbackInfo<v8::Value>& args);
727+
static void SetGracefulClose(const v8::FunctionCallbackInfo<v8::Value>& args);
726728

727729
template <get_setting fn, bool local>
728730
static void RefreshSettings(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -735,6 +737,7 @@ class Http2Session : public AsyncWrap,
735737

736738
BaseObjectPtr<Http2Ping> PopPing();
737739
bool AddPing(const uint8_t* data, v8::Local<v8::Function> callback);
740+
bool HasPendingData() const;
738741

739742
BaseObjectPtr<Http2Settings> PopSettings();
740743
bool AddSettings(v8::Local<v8::Function> callback);
@@ -785,6 +788,13 @@ class Http2Session : public AsyncWrap,
785788

786789
Statistics statistics_ = {};
787790

791+
bool IsGracefulCloseInitiated() const {
792+
return graceful_close_initiated_;
793+
}
794+
void SetGracefulCloseInitiated(bool value) {
795+
graceful_close_initiated_ = value;
796+
}
797+
788798
private:
789799
void EmitStatistics();
790800

@@ -951,8 +961,13 @@ class Http2Session : public AsyncWrap,
951961
void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length);
952962
void ClearOutgoing(int status);
953963

964+
void MaybeNotifyGracefulCloseComplete();
965+
954966
friend class Http2Scope;
955967
friend class Http2StreamListener;
968+
969+
// Flag to indicate that JavaScript has initiated a graceful closure
970+
bool graceful_close_initiated_ = false;
956971
};
957972

958973
struct Http2SessionPerformanceEntryTraits {
Collapse file

‎test/parallel/test-http2-client-rststream-before-connect.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http2-client-rststream-before-connect.js
+11-6Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,23 @@ if (!common.hasCrypto)
55
common.skip('missing crypto');
66
const assert = require('assert');
77
const h2 = require('http2');
8+
let client;
89

910
const server = h2.createServer();
1011
server.on('stream', (stream) => {
11-
stream.on('close', common.mustCall());
12-
stream.respond();
13-
stream.end('ok');
12+
stream.on('close', common.mustCall(() => {
13+
client.close();
14+
server.close();
15+
}));
16+
stream.on('error', common.expectsError({
17+
code: 'ERR_HTTP2_STREAM_ERROR',
18+
name: 'Error',
19+
message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR'
20+
}));
1421
});
1522

1623
server.listen(0, common.mustCall(() => {
17-
const client = h2.connect(`http://localhost:${server.address().port}`);
24+
client = h2.connect(`http://localhost:${server.address().port}`);
1825
const req = client.request();
1926
const closeCode = 1;
2027

@@ -52,8 +59,6 @@ server.listen(0, common.mustCall(() => {
5259
req.on('close', common.mustCall(() => {
5360
assert.strictEqual(req.destroyed, true);
5461
assert.strictEqual(req.rstCode, closeCode);
55-
server.close();
56-
client.close();
5762
}));
5863

5964
req.on('error', common.expectsError({
Collapse file

‎test/parallel/test-http2-session-graceful-close.js‎

Copy file name to clipboard
+48Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const h2 = require('http2');
8+
9+
const server = h2.createServer();
10+
let session;
11+
12+
server.on('session', common.mustCall(function(s) {
13+
session = s;
14+
session.on('close', common.mustCall(function() {
15+
server.close();
16+
}));
17+
}));
18+
19+
server.listen(0, common.mustCall(function() {
20+
const port = server.address().port;
21+
22+
const url = `http://localhost:${port}`;
23+
const client = h2.connect(url, common.mustCall(function() {
24+
const headers = {
25+
':path': '/',
26+
':method': 'GET',
27+
':scheme': 'http',
28+
':authority': `localhost:${port}`
29+
};
30+
const request = client.request(headers);
31+
request.on('response', common.mustCall(function(headers) {
32+
assert.strictEqual(headers[':status'], 200);
33+
}, 1));
34+
request.on('end', common.mustCall(function() {
35+
client.close();
36+
}));
37+
request.end();
38+
request.resume();
39+
}));
40+
client.on('goaway', common.mustCallAtLeast(1));
41+
}));
42+
43+
server.once('request', common.mustCall(function(request, response) {
44+
response.on('finish', common.mustCall(function() {
45+
session.close();
46+
}));
47+
response.end();
48+
}));

0 commit comments

Comments
0 (0)
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.