Skip to content

http2: handful of http2 src cleanups #14825

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

Closed
jasnell wants to merge 4 commits into from
Closed

http2: handful of http2 src cleanups #14825

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e07d3f4
http2: inline more stuff. remove a node_http2_core.cc
jasnell Aug 14, 2017
e80e2e6
http2: clean up debug messages
jasnell Aug 14, 2017
a536572
http2: simplify options code, and cleanup
jasnell Aug 14, 2017
cdaef97
[Squash] Don't use a macro
jasnell Aug 16, 2017
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
Prev Previous commit
Next Next commit
http2: clean up debug messages 
show server name type rather than number for more useful and
clear debug messages
  • Loading branch information
@jasnell
jasnell committed Aug 16, 2017
commit e80e2e60c237688d68f5770f0114296d0bbdcbc1
99 changes: 52 additions & 47 deletions src/node_http2_core-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ inline int Nghttp2Session::OnNghttpError(nghttp2_session* session,
size_t len,
void* user_data) {
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
DEBUG_HTTP2("Nghttp2Session %d: Error '%.*s'\n",
handle->session_type_, len, message);
DEBUG_HTTP2("Nghttp2Session %s: Error '%.*s'\n",
SESSION_TYPE_NAME(handle), len, message);
return 0;
}
#endif
Expand All @@ -55,8 +55,8 @@ inline int Nghttp2Session::OnBeginHeadersCallback(nghttp2_session* session,
int32_t id = (frame->hd.type == NGHTTP2_PUSH_PROMISE) ?
frame->push_promise.promised_stream_id :
frame->hd.stream_id;
DEBUG_HTTP2("Nghttp2Session %d: beginning headers for stream %d\n",
handle->session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: beginning headers for stream %d\n",
SESSION_TYPE_NAME(handle), id);

Nghttp2Stream* stream = handle->FindStream(id);
if (stream == nullptr) {
Expand Down Expand Up @@ -98,8 +98,8 @@ inline int Nghttp2Session::OnFrameReceive(nghttp2_session* session,
const nghttp2_frame* frame,
void* user_data) {
Nghttp2Session* handle = static_cast<Nghttp2Session*>(user_data);
DEBUG_HTTP2("Nghttp2Session %d: complete frame received: type: %d\n",
handle->session_type_, frame->hd.type);
DEBUG_HTTP2("Nghttp2Session %s: complete frame received: type: %d\n",
SESSION_TYPE_NAME(handle), frame->hd.type);
bool ack;
switch (frame->hd.type) {
case NGHTTP2_DATA:
Expand Down Expand Up @@ -130,8 +130,8 @@ inline int Nghttp2Session::OnFrameNotSent(nghttp2_session *session,
int error_code,
void *user_data) {
Nghttp2Session *handle = static_cast<Nghttp2Session *>(user_data);
DEBUG_HTTP2("Nghttp2Session %d: frame type %d was not sent, code: %d\n",
handle->session_type_, frame->hd.type, error_code);
DEBUG_HTTP2("Nghttp2Session %s: frame type %d was not sent, code: %d\n",
SESSION_TYPE_NAME(handle), frame->hd.type, error_code);
// Do not report if the frame was not sent due to the session closing
if (error_code != NGHTTP2_ERR_SESSION_CLOSING &&
error_code != NGHTTP2_ERR_STREAM_CLOSED &&
Expand All @@ -147,8 +147,8 @@ inline int Nghttp2Session::OnStreamClose(nghttp2_session *session,
uint32_t code,
void *user_data) {
Nghttp2Session *handle = static_cast<Nghttp2Session *>(user_data);
DEBUG_HTTP2("Nghttp2Session %d: stream %d closed, code: %d\n",
handle->session_type_, id, code);
DEBUG_HTTP2("Nghttp2Session %s: stream %d closed, code: %d\n",
SESSION_TYPE_NAME(handle), id, code);
Nghttp2Stream *stream = handle->FindStream(id);
// Intentionally ignore the callback if the stream does not exist
if (stream != nullptr)
Expand All @@ -167,8 +167,8 @@ inline ssize_t Nghttp2Session::OnStreamReadFD(nghttp2_session *session,
nghttp2_data_source *source,
void *user_data) {
Nghttp2Session *handle = static_cast<Nghttp2Session *>(user_data);
DEBUG_HTTP2("Nghttp2Session %d: reading outbound file data for stream %d\n",
handle->session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: reading outbound file data for stream %d\n",
SESSION_TYPE_NAME(handle), id);
Nghttp2Stream *stream = handle->FindStream(id);

int fd = source->fd;
Expand Down Expand Up @@ -203,8 +203,8 @@ inline ssize_t Nghttp2Session::OnStreamReadFD(nghttp2_session *session,

// if numchars < length, assume that we are done.
if (static_cast<size_t>(numchars) < length || length <= 0) {
DEBUG_HTTP2("Nghttp2Session %d: no more data for stream %d\n",
handle->session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: no more data for stream %d\n",
SESSION_TYPE_NAME(handle), id);
*flags |= NGHTTP2_DATA_FLAG_EOF;
GetTrailers(session, handle, stream, flags);
}
Expand All @@ -223,8 +223,8 @@ inline ssize_t Nghttp2Session::OnStreamRead(nghttp2_session *session,
nghttp2_data_source *source,
void *user_data) {
Nghttp2Session *handle = static_cast<Nghttp2Session *>(user_data);
DEBUG_HTTP2("Nghttp2Session %d: reading outbound data for stream %d\n",
handle->session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: reading outbound data for stream %d\n",
SESSION_TYPE_NAME(handle), id);
Nghttp2Stream *stream = handle->FindStream(id);
size_t remaining = length;
size_t offset = 0;
Expand All @@ -233,8 +233,8 @@ inline ssize_t Nghttp2Session::OnStreamRead(nghttp2_session *session,
// There may be data left over, which will be sent the next time nghttp
// calls this callback.
while (stream->queue_head_ != nullptr) {
DEBUG_HTTP2("Nghttp2Session %d: processing outbound data chunk\n",
handle->session_type_);
DEBUG_HTTP2("Nghttp2Session %s: processing outbound data chunk\n",
SESSION_TYPE_NAME(handle));
nghttp2_stream_write_queue *head = stream->queue_head_;
while (stream->queue_head_index_ < head->nbufs) {
if (remaining == 0)
Expand Down Expand Up @@ -274,13 +274,13 @@ inline ssize_t Nghttp2Session::OnStreamRead(nghttp2_session *session,
// again to get the data for the next DATA frame.
int writable = stream->queue_head_ != nullptr || stream->IsWritable();
if (offset == 0 && writable && stream->queue_head_ == nullptr) {
DEBUG_HTTP2("Nghttp2Session %d: deferring stream %d\n",
handle->session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: deferring stream %d\n",
SESSION_TYPE_NAME(handle), id);
return NGHTTP2_ERR_DEFERRED;
}
if (!writable) {
DEBUG_HTTP2("Nghttp2Session %d: no more data for stream %d\n",
handle->session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: no more data for stream %d\n",
SESSION_TYPE_NAME(handle), id);
*flags |= NGHTTP2_DATA_FLAG_EOF;

GetTrailers(session, handle, stream, flags);
Expand All @@ -298,8 +298,8 @@ inline ssize_t Nghttp2Session::OnSelectPadding(nghttp2_session *session,
Nghttp2Session *handle = static_cast<Nghttp2Session *>(user_data);
assert(handle->HasGetPaddingCallback());
ssize_t padding = handle->GetPadding(frame->hd.length, maxPayloadLen);
DEBUG_HTTP2("Nghttp2Session %d: using padding, size: %d\n",
handle->session_type_, padding);
DEBUG_HTTP2("Nghttp2Session %s: using padding, size: %d\n",
SESSION_TYPE_NAME(handle), padding);
return padding;
}

Expand All @@ -311,9 +311,8 @@ inline int Nghttp2Session::OnDataChunkReceived(nghttp2_session *session,
size_t len,
void *user_data) {
Nghttp2Session *handle = static_cast<Nghttp2Session *>(user_data);
DEBUG_HTTP2("Nghttp2Session %d: buffering data chunk for stream %d, size: "
"%d, flags: %d\n",
handle->session_type_, id, len, flags);
DEBUG_HTTP2("Nghttp2Session %s: buffering data chunk for stream %d, size: "
"%d, flags: %d\n", SESSION_TYPE_NAME(handle), id, len, flags);
Nghttp2Stream *stream = handle->FindStream(id);
nghttp2_data_chunk_t *chunk = data_chunk_free_list.pop();
chunk->buf = uv_buf_init(new char[len], len);
Expand Down Expand Up @@ -346,10 +345,8 @@ inline void Nghttp2Session::SubmitTrailers::Submit(nghttp2_nv *trailers,
size_t length) const {
if (length == 0)
return;
DEBUG_HTTP2("Nghttp2Session %d: sending trailers for stream %d, "
"count: %d\n",
handle_->session_type_, stream_->id(),
length);
DEBUG_HTTP2("Nghttp2Session %s: sending trailers for stream %d, "
"count: %d\n", SESSION_TYPE_NAME(handle_), stream_->id(), length);
*flags_ |= NGHTTP2_DATA_FLAG_NO_END_STREAM;
nghttp2_submit_trailer(handle_->session_,
stream_->id(),
Expand All @@ -359,7 +356,8 @@ inline void Nghttp2Session::SubmitTrailers::Submit(nghttp2_nv *trailers,

// See: https://nghttp2.org/documentation/nghttp2_submit_shutdown_notice.html
inline void Nghttp2Session::SubmitShutdownNotice() {
DEBUG_HTTP2("Nghttp2Session %d: submitting shutdown notice\n", session_type_);
DEBUG_HTTP2("Nghttp2Session %s: submitting shutdown notice\n",
SESSION_TYPE_NAME(this));
nghttp2_submit_shutdown_notice(session_);
}

Expand All @@ -368,19 +366,21 @@ inline void Nghttp2Session::SubmitShutdownNotice() {
// are no settings entries to send.
inline int Nghttp2Session::SubmitSettings(const nghttp2_settings_entry iv[],
size_t niv) {
DEBUG_HTTP2("Nghttp2Session %d: submitting settings, count: %d\n",
session_type_, niv);
DEBUG_HTTP2("Nghttp2Session %s: submitting settings, count: %d\n",
SESSION_TYPE_NAME(this), niv);
return nghttp2_submit_settings(session_, NGHTTP2_FLAG_NONE, iv, niv);
}

// Returns the Nghttp2Stream associated with the given id, or nullptr if none
inline Nghttp2Stream* Nghttp2Session::FindStream(int32_t id) {
auto s = streams_.find(id);
if (s != streams_.end()) {
DEBUG_HTTP2("Nghttp2Session %d: stream %d found\n", session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: stream %d found\n",
SESSION_TYPE_NAME(this), id);
return s->second;
} else {
DEBUG_HTTP2("Nghttp2Session %d: stream %d not found\n", session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: stream %d not found\n",
SESSION_TYPE_NAME(this), id);
return nullptr;
}
}
Expand All @@ -404,8 +404,8 @@ inline void Nghttp2Stream::FlushDataChunks(bool done) {
// to the JS side only when the frame is fully processed.
inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
int32_t id = frame->hd.stream_id;
DEBUG_HTTP2("Nghttp2Session %d: handling data frame for stream %d\n",
session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: handling data frame for stream %d\n",
SESSION_TYPE_NAME(this), id);
Nghttp2Stream* stream = this->FindStream(id);
// If the stream does not exist, something really bad happened
CHECK_NE(stream, nullptr);
Expand All @@ -420,8 +420,8 @@ inline void Nghttp2Session::HandleDataFrame(const nghttp2_frame* frame) {
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;
DEBUG_HTTP2("Nghttp2Session %d: handling headers frame for stream %d\n",
session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: handling headers frame for stream %d\n",
SESSION_TYPE_NAME(this), id);
Nghttp2Stream* stream = FindStream(id);
// If the stream does not exist, something really bad happened
CHECK_NE(stream, nullptr);
Expand All @@ -436,8 +436,8 @@ inline void Nghttp2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
inline void Nghttp2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
nghttp2_priority priority_frame = frame->priority;
int32_t id = frame->hd.stream_id;
DEBUG_HTTP2("Nghttp2Session %d: handling priority frame for stream %d\n",
session_type_, id);
DEBUG_HTTP2("Nghttp2Session %s: handling priority frame for stream %d\n",
SESSION_TYPE_NAME(this), id);
// Ignore the priority frame if stream ID is <= 0
// This actually should never happen because nghttp2 should treat this as
// an error condition that terminates the session.
Expand All @@ -450,7 +450,8 @@ inline void Nghttp2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
// Notifies the JS layer that a GOAWAY frame has been received
inline void Nghttp2Session::HandleGoawayFrame(const nghttp2_frame* frame) {
nghttp2_goaway goaway_frame = frame->goaway;
DEBUG_HTTP2("Nghttp2Session %d: handling goaway frame\n", session_type_);
DEBUG_HTTP2("Nghttp2Session %s: handling goaway frame\n",
SESSION_TYPE_NAME(this));

OnGoAway(goaway_frame.last_stream_id,
goaway_frame.error_code,
Expand All @@ -460,7 +461,8 @@ inline void Nghttp2Session::HandleGoawayFrame(const nghttp2_frame* frame) {

// Prompts nghttp2 to flush the queue of pending data frames
inline void Nghttp2Session::SendPendingData() {
DEBUG_HTTP2("Nghttp2Session %d: Sending pending data\n", session_type_);
DEBUG_HTTP2("Nghttp2Session %s: Sending pending data\n",
SESSION_TYPE_NAME(this));
// Do not attempt to send data on the socket if the destroying flag has
// been set. That means everything is shutting down and the socket
// will not be usable.
Expand Down Expand Up @@ -494,7 +496,8 @@ inline int Nghttp2Session::Init(uv_loop_t* loop,
const nghttp2_session_type type,
nghttp2_option* options,
nghttp2_mem* mem) {
DEBUG_HTTP2("Nghttp2Session %d: initializing session\n", type);
DEBUG_HTTP2("Nghttp2Session %s: initializing session\n",
SessionTypeName(type));
loop_ = loop;
session_type_ = type;
destroying_ = false;
Expand Down Expand Up @@ -548,7 +551,8 @@ inline void Nghttp2Session::MarkDestroying() {

inline int Nghttp2Session::Free() {
assert(session_ != nullptr);
DEBUG_HTTP2("Nghttp2Session %d: freeing session\n", session_type_);
DEBUG_HTTP2("Nghttp2Session %s: freeing session\n",
SESSION_TYPE_NAME(this));
// Stop the loop
uv_prepare_stop(&prep_);
auto PrepClose = [](uv_handle_t* handle) {
Expand All @@ -562,7 +566,8 @@ inline int Nghttp2Session::Free() {
nghttp2_session_del(session_);
session_ = nullptr;
loop_ = nullptr;
DEBUG_HTTP2("Nghttp2Session %d: session freed\n", session_type_);
DEBUG_HTTP2("Nghttp2Session %s: session freed\n",
SESSION_TYPE_NAME(this));
return 1;
}

Expand Down
12 changes: 12 additions & 0 deletions src/node_http2_core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ enum nghttp2_session_type {
NGHTTP2_SESSION_CLIENT
};

static inline const char* SessionTypeName(nghttp2_session_type type) {
switch (type) {
case NGHTTP2_SESSION_SERVER: return "server";
case NGHTTP2_SESSION_CLIENT: return "client";
default:
// This should never happen
ABORT();
}
}

#define SESSION_TYPE_NAME(session) SessionTypeName(session->session_type_)
Copy link
Copy Markdown
Member

@addaleax addaleax Aug 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just make this a method on Nghttp2Session? That seems easier, plus it avoids needing a #define

(If you want to stick with the define: Parentheses around session ;))


enum nghttp2_shutdown_flags {
NGHTTP2_SHUTDOWN_FLAG_GRACEFUL
};
Expand Down