Skip to content

Commit

Permalink
http2: replace direct array usage with struct for js_fields_
Browse files Browse the repository at this point in the history
PR-URL: #30534
Fixes: #30505
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
lundibundi authored and addaleax committed Nov 30, 2019
1 parent 3475f9b commit e92afd9
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
16 changes: 9 additions & 7 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,9 @@ Http2Session::Http2Session(Environment* env,
{
// Make the js_fields_ property accessible to JS land.
Local<ArrayBuffer> ab =
ArrayBuffer::New(env->isolate(), js_fields_, kSessionUint8FieldCount);
ArrayBuffer::New(env->isolate(),
reinterpret_cast<uint8_t*>(&js_fields_),
kSessionUint8FieldCount);
Local<Uint8Array> uint8_arr =
Uint8Array::New(ab, 0, kSessionUint8FieldCount);
USE(wrap->Set(env->context(), env->fields_string(), uint8_arr));
Expand Down Expand Up @@ -1046,7 +1048,7 @@ int Http2Session::OnFrameNotSent(nghttp2_session* handle,
if (error_code == NGHTTP2_ERR_SESSION_CLOSING ||
error_code == NGHTTP2_ERR_STREAM_CLOSED ||
error_code == NGHTTP2_ERR_STREAM_CLOSING ||
session->js_fields_[kSessionFrameErrorListenerCount] == 0) {
session->js_fields_.frame_error_listener_count == 0) {
return 0;
}

Expand Down Expand Up @@ -1349,7 +1351,7 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
// are considered advisory only, so this has no real effect other than to
// simply let user code know that the priority has changed.
void Http2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
if (js_fields_[kSessionPriorityListenerCount] == 0) return;
if (js_fields_.priority_listener_count == 0) return;
Isolate* isolate = env()->isolate();
HandleScope scope(isolate);
Local<Context> context = env()->context();
Expand Down Expand Up @@ -1418,7 +1420,7 @@ void Http2Session::HandleGoawayFrame(const nghttp2_frame* frame) {

// Called by OnFrameReceived when a complete ALTSVC frame has been received.
void Http2Session::HandleAltSvcFrame(const nghttp2_frame* frame) {
if (!(js_fields_[kBitfield] & (1 << kSessionHasAltsvcListeners))) return;
if (!(js_fields_.bitfield & (1 << kSessionHasAltsvcListeners))) return;
Isolate* isolate = env()->isolate();
HandleScope scope(isolate);
Local<Context> context = env()->context();
Expand Down Expand Up @@ -1497,7 +1499,7 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
return;
}

if (!(js_fields_[kBitfield] & (1 << kSessionHasPingListeners))) return;
if (!(js_fields_.bitfield & (1 << kSessionHasPingListeners))) return;
// Notify the session that a ping occurred
arg = Buffer::Copy(env(),
reinterpret_cast<const char*>(frame->ping.opaque_data),
Expand All @@ -1509,8 +1511,8 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
bool ack = frame->hd.flags & NGHTTP2_FLAG_ACK;
if (!ack) {
js_fields_[kBitfield] &= ~(1 << kSessionRemoteSettingsIsUpToDate);
if (!(js_fields_[kBitfield] & (1 << kSessionHasRemoteSettingsListeners)))
js_fields_.bitfield &= ~(1 << kSessionRemoteSettingsIsUpToDate);
if (!(js_fields_.bitfield & (1 << kSessionHasRemoteSettingsListeners)))
return;
// This is not a SETTINGS acknowledgement, notify and return
MakeCallback(env()->http2session_on_settings_function(), 0, nullptr);
Expand Down
18 changes: 13 additions & 5 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -673,15 +673,23 @@ class Http2Stream::Provider::Stream : public Http2Stream::Provider {
void* user_data);
};

typedef struct {
uint8_t bitfield;
uint8_t priority_listener_count;
uint8_t frame_error_listener_count;
} SessionJSFields;

// Indices for js_fields_, which serves as a way to communicate data with JS
// land fast. In particular, we store information about the number/presence
// of certain event listeners in JS, and skip calls from C++ into JS if they
// are missing.
enum SessionUint8Fields {
kBitfield, // See below
kSessionPriorityListenerCount,
kSessionFrameErrorListenerCount,
kSessionUint8FieldCount
kBitfield = offsetof(SessionJSFields, bitfield), // See below
kSessionPriorityListenerCount =
offsetof(SessionJSFields, priority_listener_count),
kSessionFrameErrorListenerCount =
offsetof(SessionJSFields, frame_error_listener_count),
kSessionUint8FieldCount = sizeof(SessionJSFields)
};

enum SessionBitfieldFlags {
Expand Down Expand Up @@ -968,7 +976,7 @@ class Http2Session : public AsyncWrap, public StreamListener {
nghttp2_session* session_;

// JS-accessible numeric fields, as indexed by SessionUint8Fields.
uint8_t js_fields_[kSessionUint8FieldCount] = {};
SessionJSFields js_fields_ = {};

// The session type: client or server
nghttp2_session_type session_type_;
Expand Down

0 comments on commit e92afd9

Please sign in to comment.