Skip to content

Commit d74184c

Browse files
jasnelljasnell
committed
http2: some general code improvements
PR-URL: nodejs#19400 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 224941b commit d74184c

1 file changed

Lines changed: 21 additions & 49 deletions

File tree

‎src/node_http2.cc‎

Lines changed: 21 additions & 49 deletions
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
}
@@ -1247,13 +1230,8 @@ void Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
12471230
DEBUG_HTTP2SESSION2(this, "handling data frame for stream %d", id);
12481231
Http2Stream* stream = FindStream(id);
12491232

1250-
// If the stream has already been destroyed, do nothing
1251-
if (stream->IsDestroyed())
1252-
return;
1253-
1254-
if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
1233+
if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM)
12551234
stream->EmitRead(UV_EOF);
1256-
}
12571235
}
12581236

12591237

@@ -1328,11 +1306,8 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
13281306
HandleScope scope(isolate);
13291307
Local<Context> context = env()->context();
13301308
Context::Scope context_scope(context);
1331-
1332-
Local<Value> argv[1] = {
1333-
Integer::New(isolate, NGHTTP2_ERR_PROTO),
1334-
};
1335-
MakeCallback(env()->error_string(), arraysize(argv), argv);
1309+
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
1310+
MakeCallback(env()->error_string(), 1, &arg);
13361311
}
13371312
}
13381313
}
@@ -1360,11 +1335,8 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
13601335
HandleScope scope(isolate);
13611336
Local<Context> context = env()->context();
13621337
Context::Scope context_scope(context);
1363-
1364-
Local<Value> argv[1] = {
1365-
Integer::New(isolate, NGHTTP2_ERR_PROTO),
1366-
};
1367-
MakeCallback(env()->error_string(), arraysize(argv), argv);
1338+
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
1339+
MakeCallback(env()->error_string(), 1, &arg);
13681340
}
13691341
} else {
13701342
// Otherwise, notify the session about a new settings
@@ -1787,7 +1759,7 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
17871759
{
17881760
Http2Scope h2scope(this);
17891761
flags_ |= NGHTTP2_STREAM_FLAG_SHUT;
1790-
CHECK_NE(nghttp2_session_resume_data(session_->session(), id_),
1762+
CHECK_NE(nghttp2_session_resume_data(**session_, id_),
17911763
NGHTTP2_ERR_NOMEM);
17921764
DEBUG_HTTP2STREAM(this, "writable side shutdown");
17931765
}
@@ -1848,7 +1820,7 @@ int Http2Stream::SubmitResponse(nghttp2_nv* nva, size_t len, int options) {
18481820
options |= STREAM_OPTION_EMPTY_PAYLOAD;
18491821

18501822
Http2Stream::Provider::Stream prov(this, options);
1851-
int ret = nghttp2_submit_response(session_->session(), id_, nva, len, *prov);
1823+
int ret = nghttp2_submit_response(**session_, id_, nva, len, *prov);
18521824
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
18531825
return ret;
18541826
}
@@ -1859,7 +1831,7 @@ int Http2Stream::SubmitInfo(nghttp2_nv* nva, size_t len) {
18591831
CHECK(!this->IsDestroyed());
18601832
Http2Scope h2scope(this);
18611833
DEBUG_HTTP2STREAM2(this, "sending %d informational headers", len);
1862-
int ret = nghttp2_submit_headers(session_->session(),
1834+
int ret = nghttp2_submit_headers(**session_,
18631835
NGHTTP2_FLAG_NONE,
18641836
id_, nullptr,
18651837
nva, len, nullptr);
@@ -1874,9 +1846,9 @@ int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec,
18741846
Http2Scope h2scope(this);
18751847
DEBUG_HTTP2STREAM(this, "sending priority spec");
18761848
int ret = silent ?
1877-
nghttp2_session_change_stream_priority(session_->session(),
1849+
nghttp2_session_change_stream_priority(**session_,
18781850
id_, prispec) :
1879-
nghttp2_submit_priority(session_->session(),
1851+
nghttp2_submit_priority(**session_,
18801852
NGHTTP2_FLAG_NONE,
18811853
id_, prispec);
18821854
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
@@ -1926,7 +1898,7 @@ int Http2Stream::ReadStart() {
19261898

19271899
// Tell nghttp2 about our consumption of the data that was handed
19281900
// off to JS land.
1929-
nghttp2_session_consume_stream(session_->session(),
1901+
nghttp2_session_consume_stream(**session_,
19301902
id_,
19311903
inbound_consumed_data_while_paused_);
19321904
inbound_consumed_data_while_paused_ = 0;

0 commit comments

Comments
 (0)