Skip to content

Commit 3bf69cd

Browse files
jasnellMylesBorins
authored andcommitted
http2: some general code improvements
PR-URL: #19400 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 4277635 commit 3bf69cd

File tree

1 file changed

+21
-49
lines changed

1 file changed

+21
-49
lines changed

src/node_http2.cc

+21-49
Original file line numberDiff line numberDiff line change
@@ -595,9 +595,8 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
595595
Http2Scope h2scope(this);
596596
DEBUG_HTTP2SESSION2(this, "terminating session with code %d", code);
597597
CHECK_EQ(nghttp2_session_terminate_session(session_, code), 0);
598-
} else {
599-
if (stream_ != nullptr)
600-
stream_->RemoveStreamListener(this);
598+
} else if (stream_ != nullptr) {
599+
stream_->RemoveStreamListener(this);
601600
}
602601

603602
// If there are outstanding pings, those will need to be canceled, do
@@ -688,10 +687,6 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
688687
Local<Context> context = env()->context();
689688
Context::Scope context_scope(context);
690689

691-
#if defined(DEBUG) && DEBUG
692-
CHECK(object()->Has(context, env()->ongetpadding_string()).FromJust());
693-
#endif
694-
695690
AliasedBuffer<uint32_t, v8::Uint32Array>& buffer =
696691
env()->http2_state()->padding_buffer;
697692
buffer[PADDING_BUF_FRAME_LENGTH] = frameLen;
@@ -760,18 +755,14 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
760755

761756
Http2Stream* stream = session->FindStream(id);
762757
if (stream == nullptr) {
763-
if (session->CanAddStream()) {
764-
new Http2Stream(session, id, frame->headers.cat);
765-
} else {
758+
if (!session->CanAddStream()) {
766759
// Too many concurrent streams being opened
767760
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
768761
NGHTTP2_ENHANCE_YOUR_CALM);
769762
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
770763
}
771-
} else {
772-
// If the stream has already been destroyed, ignore.
773-
if (stream->IsDestroyed())
774-
return 0;
764+
new Http2Stream(session, id, frame->headers.cat);
765+
} else if (!stream->IsDestroyed()) {
775766
stream->StartHeaders(frame->headers.cat);
776767
}
777768
return 0;
@@ -791,9 +782,7 @@ int Http2Session::OnHeaderCallback(nghttp2_session* handle,
791782
Http2Stream* stream = session->FindStream(id);
792783
CHECK_NE(stream, nullptr);
793784
// If the stream has already been destroyed, ignore.
794-
if (stream->IsDestroyed())
795-
return 0;
796-
if (!stream->AddHeader(name, value, flags)) {
785+
if (!stream->IsDestroyed() && !stream->AddHeader(name, value, flags)) {
797786
// This will only happen if the connected peer sends us more
798787
// than the allowed number of header items at any given time
799788
stream->SubmitRstStream(NGHTTP2_ENHANCE_YOUR_CALM);
@@ -859,11 +848,8 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
859848
HandleScope scope(isolate);
860849
Local<Context> context = env->context();
861850
Context::Scope context_scope(context);
862-
863-
Local<Value> argv[1] = {
864-
Integer::New(isolate, lib_error_code),
865-
};
866-
session->MakeCallback(env->error_string(), arraysize(argv), argv);
851+
Local<Value> arg = Integer::New(isolate, lib_error_code);
852+
session->MakeCallback(env->error_string(), 1, &arg);
867853
}
868854
return 0;
869855
}
@@ -1071,11 +1057,8 @@ int Http2Session::OnNghttpError(nghttp2_session* handle,
10711057
HandleScope scope(isolate);
10721058
Local<Context> context = env->context();
10731059
Context::Scope context_scope(context);
1074-
1075-
Local<Value> argv[1] = {
1076-
Integer::New(isolate, NGHTTP2_ERR_PROTO),
1077-
};
1078-
session->MakeCallback(env->error_string(), arraysize(argv), argv);
1060+
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
1061+
session->MakeCallback(env->error_string(), 1, &arg);
10791062
}
10801063
return 0;
10811064
}
@@ -1245,13 +1228,8 @@ void Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
12451228
DEBUG_HTTP2SESSION2(this, "handling data frame for stream %d", id);
12461229
Http2Stream* stream = FindStream(id);
12471230

1248-
// If the stream has already been destroyed, do nothing
1249-
if (stream->IsDestroyed())
1250-
return;
1251-
1252-
if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
1231+
if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM)
12531232
stream->EmitRead(UV_EOF);
1254-
}
12551233
}
12561234

12571235

@@ -1326,11 +1304,8 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
13261304
HandleScope scope(isolate);
13271305
Local<Context> context = env()->context();
13281306
Context::Scope context_scope(context);
1329-
1330-
Local<Value> argv[1] = {
1331-
Integer::New(isolate, NGHTTP2_ERR_PROTO),
1332-
};
1333-
MakeCallback(env()->error_string(), arraysize(argv), argv);
1307+
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
1308+
MakeCallback(env()->error_string(), 1, &arg);
13341309
}
13351310
}
13361311
}
@@ -1358,11 +1333,8 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
13581333
HandleScope scope(isolate);
13591334
Local<Context> context = env()->context();
13601335
Context::Scope context_scope(context);
1361-
1362-
Local<Value> argv[1] = {
1363-
Integer::New(isolate, NGHTTP2_ERR_PROTO),
1364-
};
1365-
MakeCallback(env()->error_string(), arraysize(argv), argv);
1336+
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
1337+
MakeCallback(env()->error_string(), 1, &arg);
13661338
}
13671339
} else {
13681340
// Otherwise, notify the session about a new settings
@@ -1782,7 +1754,7 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
17821754
{
17831755
Http2Scope h2scope(this);
17841756
flags_ |= NGHTTP2_STREAM_FLAG_SHUT;
1785-
CHECK_NE(nghttp2_session_resume_data(session_->session(), id_),
1757+
CHECK_NE(nghttp2_session_resume_data(**session_, id_),
17861758
NGHTTP2_ERR_NOMEM);
17871759
DEBUG_HTTP2STREAM(this, "writable side shutdown");
17881760
}
@@ -1843,7 +1815,7 @@ int Http2Stream::SubmitResponse(nghttp2_nv* nva, size_t len, int options) {
18431815
options |= STREAM_OPTION_EMPTY_PAYLOAD;
18441816

18451817
Http2Stream::Provider::Stream prov(this, options);
1846-
int ret = nghttp2_submit_response(session_->session(), id_, nva, len, *prov);
1818+
int ret = nghttp2_submit_response(**session_, id_, nva, len, *prov);
18471819
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
18481820
return ret;
18491821
}
@@ -1876,7 +1848,7 @@ int Http2Stream::SubmitInfo(nghttp2_nv* nva, size_t len) {
18761848
CHECK(!this->IsDestroyed());
18771849
Http2Scope h2scope(this);
18781850
DEBUG_HTTP2STREAM2(this, "sending %d informational headers", len);
1879-
int ret = nghttp2_submit_headers(session_->session(),
1851+
int ret = nghttp2_submit_headers(**session_,
18801852
NGHTTP2_FLAG_NONE,
18811853
id_, nullptr,
18821854
nva, len, nullptr);
@@ -1891,9 +1863,9 @@ int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec,
18911863
Http2Scope h2scope(this);
18921864
DEBUG_HTTP2STREAM(this, "sending priority spec");
18931865
int ret = silent ?
1894-
nghttp2_session_change_stream_priority(session_->session(),
1866+
nghttp2_session_change_stream_priority(**session_,
18951867
id_, prispec) :
1896-
nghttp2_submit_priority(session_->session(),
1868+
nghttp2_submit_priority(**session_,
18971869
NGHTTP2_FLAG_NONE,
18981870
id_, prispec);
18991871
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
@@ -1943,7 +1915,7 @@ int Http2Stream::ReadStart() {
19431915

19441916
// Tell nghttp2 about our consumption of the data that was handed
19451917
// off to JS land.
1946-
nghttp2_session_consume_stream(session_->session(),
1918+
nghttp2_session_consume_stream(**session_,
19471919
id_,
19481920
inbound_consumed_data_while_paused_);
19491921
inbound_consumed_data_while_paused_ = 0;

0 commit comments

Comments
 (0)