Skip to content

Commit

Permalink
http2: some general code improvements
Browse files Browse the repository at this point in the history 
PR-URL: #19400
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
@jasnell @MylesBorins
jasnell authored and MylesBorins committed Mar 20, 2018
1 parent 4277635 commit 3bf69cd
Showing 1 changed file with 21 additions and 49 deletions.
70 changes: 21 additions & 49 deletions src/node_http2.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,9 +595,8 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
Http2Scope h2scope(this);
DEBUG_HTTP2SESSION2(this, "terminating session with code %d", code);
CHECK_EQ(nghttp2_session_terminate_session(session_, code), 0);
} else {
if (stream_ != nullptr)
stream_->RemoveStreamListener(this);
} else if (stream_ != nullptr) {
stream_->RemoveStreamListener(this);
}

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

#if defined(DEBUG) && DEBUG
CHECK(object()->Has(context, env()->ongetpadding_string()).FromJust());
#endif

AliasedBuffer<uint32_t, v8::Uint32Array>& buffer =
env()->http2_state()->padding_buffer;
buffer[PADDING_BUF_FRAME_LENGTH] = frameLen;
Expand Down Expand Up @@ -760,18 +755,14 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,

Http2Stream* stream = session->FindStream(id);
if (stream == nullptr) {
if (session->CanAddStream()) {
new Http2Stream(session, id, frame->headers.cat);
} else {
if (!session->CanAddStream()) {
// Too many concurrent streams being opened
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
NGHTTP2_ENHANCE_YOUR_CALM);
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
} else {
// If the stream has already been destroyed, ignore.
if (stream->IsDestroyed())
return 0;
new Http2Stream(session, id, frame->headers.cat);
} else if (!stream->IsDestroyed()) {
stream->StartHeaders(frame->headers.cat);
}
return 0;
Expand All @@ -791,9 +782,7 @@ int Http2Session::OnHeaderCallback(nghttp2_session* handle,
Http2Stream* stream = session->FindStream(id);
CHECK_NE(stream, nullptr);
// If the stream has already been destroyed, ignore.
if (stream->IsDestroyed())
return 0;
if (!stream->AddHeader(name, value, flags)) {
if (!stream->IsDestroyed() && !stream->AddHeader(name, value, flags)) {
// This will only happen if the connected peer sends us more
// than the allowed number of header items at any given time
stream->SubmitRstStream(NGHTTP2_ENHANCE_YOUR_CALM);
Expand Down Expand Up @@ -859,11 +848,8 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
HandleScope scope(isolate);
Local<Context> context = env->context();
Context::Scope context_scope(context);

Local<Value> argv[1] = {
Integer::New(isolate, lib_error_code),
};
session->MakeCallback(env->error_string(), arraysize(argv), argv);
Local<Value> arg = Integer::New(isolate, lib_error_code);
session->MakeCallback(env->error_string(), 1, &arg);
}
return 0;
}
Expand Down Expand Up @@ -1071,11 +1057,8 @@ int Http2Session::OnNghttpError(nghttp2_session* handle,
HandleScope scope(isolate);
Local<Context> context = env->context();
Context::Scope context_scope(context);

Local<Value> argv[1] = {
Integer::New(isolate, NGHTTP2_ERR_PROTO),
};
session->MakeCallback(env->error_string(), arraysize(argv), argv);
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
session->MakeCallback(env->error_string(), 1, &arg);
}
return 0;
}
Expand Down Expand Up @@ -1245,13 +1228,8 @@ void Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
DEBUG_HTTP2SESSION2(this, "handling data frame for stream %d", id);
Http2Stream* stream = FindStream(id);

// If the stream has already been destroyed, do nothing
if (stream->IsDestroyed())
return;

if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM)
stream->EmitRead(UV_EOF);
}
}


Expand Down Expand Up @@ -1326,11 +1304,8 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
HandleScope scope(isolate);
Local<Context> context = env()->context();
Context::Scope context_scope(context);

Local<Value> argv[1] = {
Integer::New(isolate, NGHTTP2_ERR_PROTO),
};
MakeCallback(env()->error_string(), arraysize(argv), argv);
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
MakeCallback(env()->error_string(), 1, &arg);
}
}
}
Expand Down Expand Up @@ -1358,11 +1333,8 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
HandleScope scope(isolate);
Local<Context> context = env()->context();
Context::Scope context_scope(context);

Local<Value> argv[1] = {
Integer::New(isolate, NGHTTP2_ERR_PROTO),
};
MakeCallback(env()->error_string(), arraysize(argv), argv);
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
MakeCallback(env()->error_string(), 1, &arg);
}
} else {
// Otherwise, notify the session about a new settings
Expand Down Expand Up @@ -1782,7 +1754,7 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
{
Http2Scope h2scope(this);
flags_ |= NGHTTP2_STREAM_FLAG_SHUT;
CHECK_NE(nghttp2_session_resume_data(session_->session(), id_),
CHECK_NE(nghttp2_session_resume_data(**session_, id_),
NGHTTP2_ERR_NOMEM);
DEBUG_HTTP2STREAM(this, "writable side shutdown");
}
Expand Down Expand Up @@ -1843,7 +1815,7 @@ int Http2Stream::SubmitResponse(nghttp2_nv* nva, size_t len, int options) {
options |= STREAM_OPTION_EMPTY_PAYLOAD;

Http2Stream::Provider::Stream prov(this, options);
int ret = nghttp2_submit_response(session_->session(), id_, nva, len, *prov);
int ret = nghttp2_submit_response(**session_, id_, nva, len, *prov);
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
return ret;
}
Expand Down Expand Up @@ -1876,7 +1848,7 @@ int Http2Stream::SubmitInfo(nghttp2_nv* nva, size_t len) {
CHECK(!this->IsDestroyed());
Http2Scope h2scope(this);
DEBUG_HTTP2STREAM2(this, "sending %d informational headers", len);
int ret = nghttp2_submit_headers(session_->session(),
int ret = nghttp2_submit_headers(**session_,
NGHTTP2_FLAG_NONE,
id_, nullptr,
nva, len, nullptr);
Expand All @@ -1891,9 +1863,9 @@ int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec,
Http2Scope h2scope(this);
DEBUG_HTTP2STREAM(this, "sending priority spec");
int ret = silent ?
nghttp2_session_change_stream_priority(session_->session(),
nghttp2_session_change_stream_priority(**session_,
id_, prispec) :
nghttp2_submit_priority(session_->session(),
nghttp2_submit_priority(**session_,
NGHTTP2_FLAG_NONE,
id_, prispec);
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
Expand Down Expand Up @@ -1943,7 +1915,7 @@ int Http2Stream::ReadStart() {

// Tell nghttp2 about our consumption of the data that was handed
// off to JS land.
nghttp2_session_consume_stream(session_->session(),
nghttp2_session_consume_stream(**session_,
id_,
inbound_consumed_data_while_paused_);
inbound_consumed_data_while_paused_ = 0;
Expand Down

0 comments on commit 3bf69cd

Please sign in to comment.