Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

Commit

Permalink
http2: move uv_prepare handle to Http2Session
Browse files Browse the repository at this point in the history
As far as I understand it, `Nghttp2Session` should not be concerned
about how its consumers handle I/O.

PR-URL: nodejs/node#16461
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
  • Loading branch information
addaleax committed Dec 7, 2017
1 parent 3b58807 commit 01e2277
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 51 deletions.
41 changes: 39 additions & 2 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,45 @@ Http2Options::Http2Options(Environment* env) {
}
}

void Http2Session::OnFreeSession() {
::delete this;

Http2Session::Http2Session(Environment* env,
Local<Object> wrap,
nghttp2_session_type type)
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTP2SESSION),
StreamBase(env) {
Wrap(object(), this);

Http2Options opts(env);

padding_strategy_ = opts.GetPaddingStrategy();

Init(type, *opts);

// For every node::Http2Session instance, there is a uv_prepare_t handle
// whose callback is triggered on every tick of the event loop. When
// run, nghttp2 is prompted to send any queued data it may have stored.
prep_ = new uv_prepare_t();
uv_prepare_init(env->event_loop(), prep_);
prep_->data = static_cast<void*>(this);
uv_prepare_start(prep_, [](uv_prepare_t* t) {
Http2Session* session = static_cast<Http2Session*>(t->data);
session->SendPendingData();
});
}

Http2Session::~Http2Session() {
CHECK_EQ(false, persistent().IsEmpty());
ClearWrap(object());
persistent().Reset();
CHECK_EQ(true, persistent().IsEmpty());

// Stop the loop
CHECK_EQ(uv_prepare_stop(prep_), 0);
auto prep_close = [](uv_handle_t* handle) {
delete reinterpret_cast<uv_prepare_t*>(handle);
};
uv_close(reinterpret_cast<uv_handle_t*>(prep_), prep_close);
prep_ = nullptr;
}

ssize_t Http2Session::OnMaxFrameSizePadding(size_t frameLen,
Expand Down
27 changes: 7 additions & 20 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -343,24 +343,8 @@ class Http2Session : public AsyncWrap,
public:
Http2Session(Environment* env,
Local<Object> wrap,
nghttp2_session_type type) :
AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTP2SESSION),
StreamBase(env) {
Wrap(object(), this);

Http2Options opts(env);

padding_strategy_ = opts.GetPaddingStrategy();

Init(env->event_loop(), type, *opts);
}

~Http2Session() override {
CHECK_EQ(false, persistent().IsEmpty());
ClearWrap(object());
persistent().Reset();
CHECK_EQ(true, persistent().IsEmpty());
}
nghttp2_session_type type);
~Http2Session() override;

static void OnStreamAllocImpl(size_t suggested_size,
uv_buf_t* buf,
Expand All @@ -369,9 +353,8 @@ class Http2Session : public AsyncWrap,
const uv_buf_t* bufs,
uv_handle_type pending,
void* ctx);
protected:
void OnFreeSession() override;

protected:
ssize_t OnMaxFrameSizePadding(size_t frameLength,
size_t maxPayloadLen);

Expand Down Expand Up @@ -449,6 +432,9 @@ class Http2Session : public AsyncWrap,
return 0;
}

uv_loop_t* event_loop() const override {
return env()->event_loop();
}
public:
void Consume(Local<External> external);
void Unconsume();
Expand Down Expand Up @@ -496,6 +482,7 @@ class Http2Session : public AsyncWrap,

// use this to allow timeout tracking during long-lasting writes
uint32_t chunks_sent_since_last_write_ = 0;
uv_prepare_t* prep_ = nullptr;

char stream_buf_[kAllocBufferSize];
};
Expand Down
29 changes: 5 additions & 24 deletions src/node_http2_core-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ inline ssize_t Nghttp2Session::OnStreamReadFD(nghttp2_session* session,
uv_fs_t read_req;

if (length > 0) {
numchars = uv_fs_read(handle->loop_,
// TODO(addaleax): Never use synchronous I/O on the main thread.
numchars = uv_fs_read(handle->event_loop(),
&read_req,
fd, &data, 1,
offset, nullptr);
Expand Down Expand Up @@ -541,11 +542,9 @@ inline void Nghttp2Session::SendPendingData() {
// Initialize the Nghttp2Session handle by creating and
// assigning the Nghttp2Session instance and associated
// uv_loop_t.
inline int Nghttp2Session::Init(uv_loop_t* loop,
const nghttp2_session_type type,
nghttp2_option* options,
nghttp2_mem* mem) {
loop_ = loop;
inline int Nghttp2Session::Init(const nghttp2_session_type type,
nghttp2_option* options,
nghttp2_mem* mem) {
session_type_ = type;
DEBUG_HTTP2("Nghttp2Session %s: initializing session\n", TypeName());
destroying_ = false;
Expand Down Expand Up @@ -581,14 +580,6 @@ inline int Nghttp2Session::Init(uv_loop_t* loop,
nghttp2_option_del(opts);
}

// For every node::Http2Session instance, there is a uv_prep_t handle
// whose callback is triggered on every tick of the event loop. When
// run, nghttp2 is prompted to send any queued data it may have stored.
uv_prepare_init(loop_, &prep_);
uv_prepare_start(&prep_, [](uv_prepare_t* t) {
Nghttp2Session* session = ContainerOf(&Nghttp2Session::prep_, t);
session->SendPendingData();
});
return ret;
}

Expand All @@ -601,19 +592,9 @@ inline int Nghttp2Session::Free() {
CHECK(session_ != nullptr);
#endif
DEBUG_HTTP2("Nghttp2Session %s: freeing session\n", TypeName());
// Stop the loop
CHECK_EQ(uv_prepare_stop(&prep_), 0);
auto PrepClose = [](uv_handle_t* handle) {
Nghttp2Session* session =
ContainerOf(&Nghttp2Session::prep_,
reinterpret_cast<uv_prepare_t*>(handle));
session->OnFreeSession();
};
uv_close(reinterpret_cast<uv_handle_t*>(&prep_), PrepClose);
nghttp2_session_terminate_session(session_, NGHTTP2_NO_ERROR);
nghttp2_session_del(session_);
session_ = nullptr;
loop_ = nullptr;
DEBUG_HTTP2("Nghttp2Session %s: session freed\n", TypeName());
return 1;
}
Expand Down
9 changes: 4 additions & 5 deletions src/node_http2_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ class Nghttp2Session {
public:
// Initializes the session instance
inline int Init(
uv_loop_t*,
const nghttp2_session_type type = NGHTTP2_SESSION_SERVER,
nghttp2_option* options = nullptr,
nghttp2_mem* mem = nullptr);
Expand Down Expand Up @@ -175,7 +174,6 @@ class Nghttp2Session {
int error_code) {}
virtual ssize_t GetPadding(size_t frameLength,
size_t maxFrameLength) { return 0; }
virtual void OnFreeSession() {}
virtual void AllocateSend(uv_buf_t* buf) = 0;

virtual bool HasGetPaddingCallback() { return false; }
Expand All @@ -199,8 +197,11 @@ class Nghttp2Session {
virtual void OnTrailers(Nghttp2Stream* stream,
const SubmitTrailers& submit_trailers) {}

private:
inline void SendPendingData();

virtual uv_loop_t* event_loop() const = 0;

private:
inline void HandleHeadersFrame(const nghttp2_frame* frame);
inline void HandlePriorityFrame(const nghttp2_frame* frame);
inline void HandleDataFrame(const nghttp2_frame* frame);
Expand Down Expand Up @@ -281,8 +282,6 @@ class Nghttp2Session {
static const Callbacks callback_struct_saved[2];

nghttp2_session* session_;
uv_loop_t* loop_;
uv_prepare_t prep_;
nghttp2_session_type session_type_;
std::unordered_map<int32_t, Nghttp2Stream*> streams_;
bool destroying_ = false;
Expand Down

0 comments on commit 01e2277

Please sign in to comment.