1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

http2: some general code improvements

Browse Source 
PR-URL: https://github.com/nodejs/node/pull/19400
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
James M Snell 2018-03-16 14:48:22 -07:00
parent 224941bd8c
commit d74184c2fa
1 changed files with 21 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/node_http2.cc
View File

@ -595,9 +595,8 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
Http2Scope h2scope(this); Http2Scope h2scope(this);
DEBUG_HTTP2SESSION2(this, "terminating session with code %d", code); DEBUG_HTTP2SESSION2(this, "terminating session with code %d", code);
CHECK_EQ(nghttp2_session_terminate_session(session_, code), 0); CHECK_EQ(nghttp2_session_terminate_session(session_, code), 0);
} else { } else if (stream_ != nullptr) {
if (stream_ != nullptr) stream_->RemoveStreamListener(this);
stream_->RemoveStreamListener(this);
} }
// If there are outstanding pings, those will need to be canceled, do // If there are outstanding pings, those will need to be canceled, do
@ -688,10 +687,6 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
Local<Context> context = env()->context(); Local<Context> context = env()->context();
Context::Scope context_scope(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 = AliasedBuffer<uint32_t, v8::Uint32Array>& buffer =
env()->http2_state()->padding_buffer; env()->http2_state()->padding_buffer;
buffer[PADDING_BUF_FRAME_LENGTH] = frameLen; buffer[PADDING_BUF_FRAME_LENGTH] = frameLen;
@ -760,18 +755,14 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
Http2Stream* stream = session->FindStream(id); Http2Stream* stream = session->FindStream(id);
if (stream == nullptr) { if (stream == nullptr) {
if (session->CanAddStream()) { if (!session->CanAddStream()) {
new Http2Stream(session, id, frame->headers.cat);
} else {
// Too many concurrent streams being opened // Too many concurrent streams being opened
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id, nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
NGHTTP2_ENHANCE_YOUR_CALM); NGHTTP2_ENHANCE_YOUR_CALM);
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
} }
} else { new Http2Stream(session, id, frame->headers.cat);
// If the stream has already been destroyed, ignore. } else if (!stream->IsDestroyed()) {
if (stream->IsDestroyed())
return 0;
stream->StartHeaders(frame->headers.cat); stream->StartHeaders(frame->headers.cat);
} }
return 0; return 0;
@ -791,9 +782,7 @@ int Http2Session::OnHeaderCallback(nghttp2_session* handle,
Http2Stream* stream = session->FindStream(id); Http2Stream* stream = session->FindStream(id);
CHECK_NE(stream, nullptr); CHECK_NE(stream, nullptr);
// If the stream has already been destroyed, ignore. // If the stream has already been destroyed, ignore.
if (stream->IsDestroyed()) if (!stream->IsDestroyed() && !stream->AddHeader(name, value, flags)) {
return 0;
if (!stream->AddHeader(name, value, flags)) {
// This will only happen if the connected peer sends us more // This will only happen if the connected peer sends us more
// than the allowed number of header items at any given time // than the allowed number of header items at any given time
stream->SubmitRstStream(NGHTTP2_ENHANCE_YOUR_CALM); stream->SubmitRstStream(NGHTTP2_ENHANCE_YOUR_CALM);
@ -859,11 +848,8 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
HandleScope scope(isolate); HandleScope scope(isolate);
Local<Context> context = env->context(); Local<Context> context = env->context();
Context::Scope context_scope(context); Context::Scope context_scope(context);
Local<Value> arg = Integer::New(isolate, lib_error_code);
Local<Value> argv[1] = { session->MakeCallback(env->error_string(), 1, &arg);
Integer::New(isolate, lib_error_code),
};
session->MakeCallback(env->error_string(), arraysize(argv), argv);
} }
return 0; return 0;
} }
@ -1071,11 +1057,8 @@ int Http2Session::OnNghttpError(nghttp2_session* handle,
HandleScope scope(isolate); HandleScope scope(isolate);
Local<Context> context = env->context(); Local<Context> context = env->context();
Context::Scope context_scope(context); Context::Scope context_scope(context);
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
Local<Value> argv[1] = { session->MakeCallback(env->error_string(), 1, &arg);
Integer::New(isolate, NGHTTP2_ERR_PROTO),
};
session->MakeCallback(env->error_string(), arraysize(argv), argv);
} }
return 0; return 0;
} }
@ -1247,13 +1230,8 @@ void Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
DEBUG_HTTP2SESSION2(this, "handling data frame for stream %d", id); DEBUG_HTTP2SESSION2(this, "handling data frame for stream %d", id);
Http2Stream* stream = FindStream(id); Http2Stream* stream = FindStream(id);
// If the stream has already been destroyed, do nothing if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM)
if (stream->IsDestroyed())
return;
if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
stream->EmitRead(UV_EOF); stream->EmitRead(UV_EOF);
}
} }
@ -1328,11 +1306,8 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
HandleScope scope(isolate); HandleScope scope(isolate);
Local<Context> context = env()->context(); Local<Context> context = env()->context();
Context::Scope context_scope(context); Context::Scope context_scope(context);
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
Local<Value> argv[1] = { MakeCallback(env()->error_string(), 1, &arg);
Integer::New(isolate, NGHTTP2_ERR_PROTO),
};
MakeCallback(env()->error_string(), arraysize(argv), argv);
} }
} }
} }
@ -1360,11 +1335,8 @@ void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
HandleScope scope(isolate); HandleScope scope(isolate);
Local<Context> context = env()->context(); Local<Context> context = env()->context();
Context::Scope context_scope(context); Context::Scope context_scope(context);
Local<Value> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
Local<Value> argv[1] = { MakeCallback(env()->error_string(), 1, &arg);
Integer::New(isolate, NGHTTP2_ERR_PROTO),
};
MakeCallback(env()->error_string(), arraysize(argv), argv);
} }
} else { } else {
// Otherwise, notify the session about a new settings // Otherwise, notify the session about a new settings
@ -1787,7 +1759,7 @@ int Http2Stream::DoShutdown(ShutdownWrap* req_wrap) {
{ {
Http2Scope h2scope(this); Http2Scope h2scope(this);
flags_ |= NGHTTP2_STREAM_FLAG_SHUT; 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); NGHTTP2_ERR_NOMEM);
DEBUG_HTTP2STREAM(this, "writable side shutdown"); DEBUG_HTTP2STREAM(this, "writable side shutdown");
} }
@ -1848,7 +1820,7 @@ int Http2Stream::SubmitResponse(nghttp2_nv* nva, size_t len, int options) {
options |= STREAM_OPTION_EMPTY_PAYLOAD; options |= STREAM_OPTION_EMPTY_PAYLOAD;
Http2Stream::Provider::Stream prov(this, options); 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); CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
return ret; return ret;
} }
@ -1859,7 +1831,7 @@ int Http2Stream::SubmitInfo(nghttp2_nv* nva, size_t len) {
CHECK(!this->IsDestroyed()); CHECK(!this->IsDestroyed());
Http2Scope h2scope(this); Http2Scope h2scope(this);
DEBUG_HTTP2STREAM2(this, "sending %d informational headers", len); DEBUG_HTTP2STREAM2(this, "sending %d informational headers", len);
int ret = nghttp2_submit_headers(session_->session(), int ret = nghttp2_submit_headers(**session_,
NGHTTP2_FLAG_NONE, NGHTTP2_FLAG_NONE,
id_, nullptr, id_, nullptr,
nva, len, nullptr); nva, len, nullptr);
@ -1874,9 +1846,9 @@ int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec,
Http2Scope h2scope(this); Http2Scope h2scope(this);
DEBUG_HTTP2STREAM(this, "sending priority spec"); DEBUG_HTTP2STREAM(this, "sending priority spec");
int ret = silent ? int ret = silent ?
nghttp2_session_change_stream_priority(session_->session(), nghttp2_session_change_stream_priority(**session_,
id_, prispec) : id_, prispec) :
nghttp2_submit_priority(session_->session(), nghttp2_submit_priority(**session_,
NGHTTP2_FLAG_NONE, NGHTTP2_FLAG_NONE,
id_, prispec); id_, prispec);
CHECK_NE(ret, NGHTTP2_ERR_NOMEM); CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
@ -1926,7 +1898,7 @@ int Http2Stream::ReadStart() {
// Tell nghttp2 about our consumption of the data that was handed // Tell nghttp2 about our consumption of the data that was handed
// off to JS land. // off to JS land.
nghttp2_session_consume_stream(session_->session(), nghttp2_session_consume_stream(**session_,
id_, id_,
inbound_consumed_data_while_paused_); inbound_consumed_data_while_paused_);
inbound_consumed_data_while_paused_ = 0; inbound_consumed_data_while_paused_ = 0;