Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: multiple smaller code cleanups #16764

Closed
wants to merge 1 commit into from
Closed

http2: multiple smaller code cleanups #16764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 22 additions & 31 deletions src/node_http2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "node_http2_state.h"

#include <queue>
#include <algorithm>

namespace node {

Expand Down Expand Up @@ -180,6 +181,18 @@ inline void Http2Settings::RefreshDefaults(Environment* env) {
(1 << IDX_SETTINGS_MAX_FRAME_SIZE);
}

Http2Priority::Http2Priority(Environment* env,
Local<Value> parent,
Local<Value> weight,
Local<Value> exclusive) {
Local<Context> context = env->context();
int32_t parent_ = parent->Int32Value(context).ToChecked();
int32_t weight_ = weight->Int32Value(context).ToChecked();
bool exclusive_ = exclusive->BooleanValue(context).ToChecked();
DEBUG_HTTP2("Http2Priority: parent: %d, weight: %d, exclusive: %d\n",
parent_, weight_, exclusive_);
nghttp2_priority_spec_init(&spec, parent_, weight_, exclusive_ ? 1 : 0);
}

Http2Session::Http2Session(Environment* env,
Local<Object> wrap,
Expand Down Expand Up @@ -258,12 +271,8 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
buffer[PADDING_BUF_RETURN_VALUE] = frameLen;
MakeCallback(env()->ongetpadding_string(), 0, nullptr);
uint32_t retval = buffer[PADDING_BUF_RETURN_VALUE];
retval = retval <= maxPayloadLen ? retval : maxPayloadLen;
retval = retval >= frameLen ? retval : frameLen;
#if defined(DEBUG) && DEBUG
CHECK_GE(retval, frameLen);
CHECK_LE(retval, maxPayloadLen);
#endif
retval = std::min(retval, static_cast<uint32_t>(maxPayloadLen));
retval = std::max(retval, static_cast<uint32_t>(frameLen));
return retval;
}

Expand Down Expand Up @@ -445,30 +454,18 @@ void Http2Session::SubmitPriority(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
Local<Context> context = env->context();

nghttp2_priority_spec spec;
int32_t id = args[0]->Int32Value(context).ToChecked();
int32_t parent = args[1]->Int32Value(context).ToChecked();
int32_t weight = args[2]->Int32Value(context).ToChecked();
bool exclusive = args[3]->BooleanValue(context).ToChecked();
Http2Priority priority(env, args[1], args[2], args[3]);
bool silent = args[4]->BooleanValue(context).ToChecked();
DEBUG_HTTP2("Http2Session: submitting priority for stream %d: "
"parent: %d, weight: %d, exclusive: %d, silent: %d\n",
id, parent, weight, exclusive, silent);

#if defined(DEBUG) && DEBUG
CHECK_GT(id, 0);
CHECK_GE(parent, 0);
CHECK_GE(weight, 0);
#endif
DEBUG_HTTP2("Http2Session: submitting priority for stream %d", id);

Nghttp2Stream* stream;
if (!(stream = session->FindStream(id))) {
// invalid stream
return args.GetReturnValue().Set(NGHTTP2_ERR_INVALID_STREAM_ID);
}
nghttp2_priority_spec_init(&spec, parent, weight, exclusive ? 1 : 0);

args.GetReturnValue().Set(stream->SubmitPriority(&spec, silent));
args.GetReturnValue().Set(stream->SubmitPriority(*priority, silent));
}

void Http2Session::SubmitSettings(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -524,20 +521,14 @@ void Http2Session::SubmitRequest(const FunctionCallbackInfo<Value>& args) {

Local<Array> headers = args[0].As<Array>();
int options = args[1]->IntegerValue(context).ToChecked();
int32_t parent = args[2]->Int32Value(context).ToChecked();
int32_t weight = args[3]->Int32Value(context).ToChecked();
bool exclusive = args[4]->BooleanValue(context).ToChecked();

DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d, "
"parent: %d, weight: %d, exclusive: %d\n", headers->Length(),
options, parent, weight, exclusive);
Http2Priority priority(env, args[2], args[3], args[4]);

nghttp2_priority_spec prispec;
nghttp2_priority_spec_init(&prispec, parent, weight, exclusive ? 1 : 0);
DEBUG_HTTP2("Http2Session: submitting request: headers: %d, options: %d\n",
headers->Length(), options);

Headers list(isolate, context, headers);

int32_t ret = session->Nghttp2Session::SubmitRequest(&prispec,
int32_t ret = session->Nghttp2Session::SubmitRequest(*priority,
*list, list.length(),
nullptr, options);
DEBUG_HTTP2("Http2Session: request submitted, response: %d\n", ret);
Expand Down
14 changes: 14 additions & 0 deletions src/node_http2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,20 @@ class Http2Settings {
MaybeStackBuffer<nghttp2_settings_entry, IDX_SETTINGS_COUNT> entries_;
};

class Http2Priority {
public:
Http2Priority(Environment* env,
Local<Value> parent,
Local<Value> weight,
Local<Value> exclusive);

nghttp2_priority_spec* operator*() {
return &spec;
}
private:
nghttp2_priority_spec spec;
};

class Http2Session : public AsyncWrap,
public StreamBase,
public Nghttp2Session {
Expand Down
63 changes: 22 additions & 41 deletions src/node_http2_core-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,21 @@ inline int Nghttp2Session::OnNghttpError(nghttp2_session* session,
}
#endif

inline int32_t GetFrameID(const nghttp2_frame* frame) {
// If this is a push promise, we want to grab the id of the promised stream
return (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id :
frame->hd.stream_id;
}

// nghttp2 calls this at the beginning a new HEADERS or PUSH_PROMISE frame.
// We use it to ensure that an Nghttp2Stream instance is allocated to store
// the state.
inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session,
const nghttp2_frame* frame,
void* user_data) {
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
// If this is a push promise frame, we want to grab the handle of
// the promised stream.
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id :
frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: beginning headers for stream %d\n",
handle->TypeName(), id);

Expand All @@ -62,11 +65,7 @@ inline int Nghttp2Session::OnHeaderCallback(nghttp2_session* session,
uint8_t flags,
void* user_data) {
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
// If this is a push promise frame, we want to grab the handle of
// the promised stream.
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id :
frame->hd.stream_id;
int32_t id = GetFrameID(frame);
Nghttp2Stream* stream = handle->FindStream(id);
// The header name and value are stored in a reference counted buffer
// provided to us by nghttp2. We need to increment the reference counter
Expand Down Expand Up @@ -418,7 +417,7 @@ inline void Nghttp2Stream::FlushDataChunks() {
// see if the END_STREAM flag is set, and will flush the queued data chunks
// to JS if the stream is flowing
inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
int32_t id = frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: handling data frame for stream %d\n",
TypeName(), id);
Nghttp2Stream* stream = this->FindStream(id);
Expand All @@ -436,8 +435,7 @@ inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
// The headers are collected as the frame is being processed and sent out
// to the JS side only when the frame is fully processed.
inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id : frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: handling headers frame for stream %d\n",
TypeName(), id);
Nghttp2Stream* stream = FindStream(id);
Expand All @@ -454,7 +452,7 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
// Notifies the JS layer that a PRIORITY frame has been received
inline void Nghttp2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
nghttp2_priority priority_frame = frame->priority;
int32_t id = frame->hd.stream_id;
int32_t id = GetFrameID(frame);
DEBUG_HTTP2("Nghttp2Session %s: handling priority frame for stream %d\n",
TypeName(), id);

Expand Down Expand Up @@ -555,39 +553,22 @@ inline int Nghttp2Session::Init(const nghttp2_session_type type,
session_type_ = type;
DEBUG_HTTP2("Nghttp2Session %s: initializing session\n", TypeName());
destroying_ = false;
int ret = 0;

nghttp2_session_callbacks* callbacks
= callback_struct_saved[HasGetPaddingCallback() ? 1 : 0].callbacks;

nghttp2_option* opts;
if (options != nullptr) {
opts = options;
} else {
nghttp2_option_new(&opts);
}
CHECK_NE(options, nullptr);

switch (type) {
case NGHTTP2_SESSION_SERVER:
ret = nghttp2_session_server_new3(&session_,
callbacks,
this,
opts,
mem);
break;
case NGHTTP2_SESSION_CLIENT:
ret = nghttp2_session_client_new3(&session_,
callbacks,
this,
opts,
mem);
break;
}
if (opts != options) {
nghttp2_option_del(opts);
}
typedef int (*init_fn)(nghttp2_session** session,
const nghttp2_session_callbacks* callbacks,
void* user_data,
const nghttp2_option* options,
nghttp2_mem* mem);
init_fn fn = type == NGHTTP2_SESSION_SERVER ?
nghttp2_session_server_new3 :
nghttp2_session_client_new3;

return ret;
return fn(&session_, callbacks, this, options, mem);
}

inline void Nghttp2Session::MarkDestroying() {
Expand Down