diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 93ae66ca890a0a..d2f1c22ac705df 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -167,6 +167,7 @@ const kID = Symbol('id'); const kInit = Symbol('init'); const kInfoHeaders = Symbol('sent-info-headers'); const kLocalSettings = Symbol('local-settings'); +const kNativeFields = Symbol('kNativeFields'); const kOptions = Symbol('options'); const kOwner = owner_symbol; const kOrigin = Symbol('origin'); @@ -188,7 +189,15 @@ const { paddingBuffer, PADDING_BUF_FRAME_LENGTH, PADDING_BUF_MAX_PAYLOAD_LENGTH, - PADDING_BUF_RETURN_VALUE + PADDING_BUF_RETURN_VALUE, + kBitfield, + kSessionPriorityListenerCount, + kSessionFrameErrorListenerCount, + kSessionUint8FieldCount, + kSessionHasRemoteSettingsListeners, + kSessionRemoteSettingsIsUpToDate, + kSessionHasPingListeners, + kSessionHasAltsvcListeners, } = binding; const { @@ -364,6 +373,76 @@ function submitRstStream(code) { } } +// Keep track of the number/presence of JS event listeners. Knowing that there +// are no listeners allows the C++ code to skip calling into JS for an event. +function sessionListenerAdded(name) { + switch (name) { + case 'ping': + this[kNativeFields][kBitfield] |= 1 << kSessionHasPingListeners; + break; + case 'altsvc': + this[kNativeFields][kBitfield] |= 1 << kSessionHasAltsvcListeners; + break; + case 'remoteSettings': + this[kNativeFields][kBitfield] |= 1 << kSessionHasRemoteSettingsListeners; + break; + case 'priority': + this[kNativeFields][kSessionPriorityListenerCount]++; + break; + case 'frameError': + this[kNativeFields][kSessionFrameErrorListenerCount]++; + break; + } +} + +function sessionListenerRemoved(name) { + switch (name) { + case 'ping': + if (this.listenerCount(name) > 0) return; + this[kNativeFields][kBitfield] &= ~(1 << kSessionHasPingListeners); + break; + case 'altsvc': + if (this.listenerCount(name) > 0) return; + this[kNativeFields][kBitfield] &= ~(1 << kSessionHasAltsvcListeners); + break; + case 'remoteSettings': + if (this.listenerCount(name) > 0) return; + this[kNativeFields][kBitfield] &= + ~(1 << kSessionHasRemoteSettingsListeners); + break; + case 'priority': + this[kNativeFields][kSessionPriorityListenerCount]--; + break; + case 'frameError': + this[kNativeFields][kSessionFrameErrorListenerCount]--; + break; + } +} + +// Also keep track of listeners for the Http2Stream instances, as some events +// are emitted on those objects. +function streamListenerAdded(name) { + switch (name) { + case 'priority': + this[kSession][kNativeFields][kSessionPriorityListenerCount]++; + break; + case 'frameError': + this[kSession][kNativeFields][kSessionFrameErrorListenerCount]++; + break; + } +} + +function streamListenerRemoved(name) { + switch (name) { + case 'priority': + this[kSession][kNativeFields][kSessionPriorityListenerCount]--; + break; + case 'frameError': + this[kSession][kNativeFields][kSessionFrameErrorListenerCount]--; + break; + } +} + function onPing(payload) { const session = this[kOwner]; if (session.destroyed) @@ -421,7 +500,6 @@ function onSettings() { return; session[kUpdateTimer](); debugSessionObj(session, 'new settings received'); - session[kRemoteSettings] = undefined; session.emit('remoteSettings', session.remoteSettings); } @@ -863,6 +941,10 @@ function setupHandle(socket, type, options) { handle.consume(socket._handle._externalStream); this[kHandle] = handle; + if (this[kNativeFields]) + handle.fields.set(this[kNativeFields]); + else + this[kNativeFields] = handle.fields; if (socket.encrypted) { this[kAlpnProtocol] = socket.alpnProtocol; @@ -904,6 +986,7 @@ function finishSessionDestroy(session, error) { session[kProxySocket] = undefined; session[kSocket] = undefined; session[kHandle] = undefined; + session[kNativeFields] = new Uint8Array(kSessionUint8FieldCount); socket[kSession] = undefined; socket[kServer] = undefined; @@ -982,6 +1065,7 @@ class Http2Session extends EventEmitter { this[kProxySocket] = null; this[kSocket] = socket; this[kTimeout] = null; + this[kHandle] = undefined; // Do not use nagle's algorithm if (typeof socket.setNoDelay === 'function') @@ -1000,6 +1084,11 @@ class Http2Session extends EventEmitter { setupFn(); } + if (!this[kNativeFields]) + this[kNativeFields] = new Uint8Array(kSessionUint8FieldCount); + this.on('newListener', sessionListenerAdded); + this.on('removeListener', sessionListenerRemoved); + debugSession(type, 'created'); } @@ -1154,13 +1243,18 @@ class Http2Session extends EventEmitter { // The settings currently in effect for the remote peer. get remoteSettings() { - const settings = this[kRemoteSettings]; - if (settings !== undefined) - return settings; + if (this[kNativeFields][kBitfield] & + (1 << kSessionRemoteSettingsIsUpToDate)) { + const settings = this[kRemoteSettings]; + if (settings !== undefined) { + return settings; + } + } if (this.destroyed || this.connecting) return {}; + this[kNativeFields][kBitfield] |= (1 << kSessionRemoteSettingsIsUpToDate); return this[kRemoteSettings] = getSettings(this[kHandle], true); // Remote } @@ -1343,6 +1437,12 @@ class ServerHttp2Session extends Http2Session { constructor(options, socket, server) { super(NGHTTP2_SESSION_SERVER, options, socket); this[kServer] = server; + // This is a bit inaccurate because it does not reflect changes to + // number of listeners made after the session was created. This should + // not be an issue in practice. Additionally, the 'priority' event on + // server instances (or any other object) is fully undocumented. + this[kNativeFields][kSessionPriorityListenerCount] = + server.listenerCount('priority'); } get server() { @@ -1660,6 +1760,9 @@ class Http2Stream extends Duplex { }; this.on('pause', streamOnPause); + + this.on('newListener', streamListenerAdded); + this.on('removeListener', streamListenerRemoved); } [kUpdateTimer]() { @@ -2633,7 +2736,7 @@ function sessionOnPriority(stream, parent, weight, exclusive) { } function sessionOnError(error) { - if (this[kServer]) + if (this[kServer] !== undefined) this[kServer].emit('sessionError', error, this); } @@ -2682,8 +2785,10 @@ function connectionListener(socket) { const session = new ServerHttp2Session(options, socket, this); session.on('stream', sessionOnStream); - session.on('priority', sessionOnPriority); session.on('error', sessionOnError); + // Don't count our own internal listener. + session.on('priority', sessionOnPriority); + session[kNativeFields][kSessionPriorityListenerCount]--; if (this.timeout) session.setTimeout(this.timeout, sessionOnTimeout); diff --git a/src/env.h b/src/env.h index d7003616ece13c..7f77face35057f 100644 --- a/src/env.h +++ b/src/env.h @@ -173,6 +173,7 @@ struct PackageConfig { V(family_string, "family") \ V(fatal_exception_string, "_fatalException") \ V(fd_string, "fd") \ + V(fields_string, "fields") \ V(file_string, "file") \ V(fingerprint256_string, "fingerprint256") \ V(fingerprint_string, "fingerprint") \ diff --git a/src/node_http2.cc b/src/node_http2.cc index c7ccaf78690f09..6882c9d957c436 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -24,6 +24,7 @@ using v8::ObjectTemplate; using v8::String; using v8::Uint32; using v8::Uint32Array; +using v8::Uint8Array; using v8::Undefined; using node::performance::PerformanceEntry; @@ -631,6 +632,15 @@ Http2Session::Http2Session(Environment* env, outgoing_storage_.reserve(4096); outgoing_buffers_.reserve(32); + + { + // Make the js_fields_ property accessible to JS land. + Local ab = + ArrayBuffer::New(env->isolate(), js_fields_, kSessionUint8FieldCount); + Local uint8_arr = + Uint8Array::New(ab, 0, kSessionUint8FieldCount); + USE(wrap->Set(env->context(), env->fields_string(), uint8_arr)); + } } Http2Session::~Http2Session() { @@ -1032,7 +1042,8 @@ int Http2Session::OnFrameNotSent(nghttp2_session* handle, // 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 || - error_code == NGHTTP2_ERR_STREAM_CLOSING) { + error_code == NGHTTP2_ERR_STREAM_CLOSING || + session->js_fields_[kSessionFrameErrorListenerCount] == 0) { return 0; } @@ -1337,6 +1348,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; Isolate* isolate = env()->isolate(); HandleScope scope(isolate); Local context = env()->context(); @@ -1399,6 +1411,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; Isolate* isolate = env()->isolate(); HandleScope scope(isolate); Local context = env()->context(); @@ -1484,6 +1497,7 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) { return; } + if (!(js_fields_[kBitfield] & (1 << kSessionHasPingListeners))) return; // Notify the session that a ping occurred arg = Buffer::Copy(env(), reinterpret_cast(frame->ping.opaque_data), @@ -1495,6 +1509,9 @@ 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))) + return; // This is not a SETTINGS acknowledgement, notify and return MakeCallback(env()->onsettings_string(), 0, nullptr); return; @@ -2980,6 +2997,16 @@ void Initialize(Local target, NODE_DEFINE_CONSTANT(target, PADDING_BUF_MAX_PAYLOAD_LENGTH); NODE_DEFINE_CONSTANT(target, PADDING_BUF_RETURN_VALUE); + NODE_DEFINE_CONSTANT(target, kBitfield); + NODE_DEFINE_CONSTANT(target, kSessionPriorityListenerCount); + NODE_DEFINE_CONSTANT(target, kSessionFrameErrorListenerCount); + NODE_DEFINE_CONSTANT(target, kSessionUint8FieldCount); + + NODE_DEFINE_CONSTANT(target, kSessionHasRemoteSettingsListeners); + NODE_DEFINE_CONSTANT(target, kSessionRemoteSettingsIsUpToDate); + NODE_DEFINE_CONSTANT(target, kSessionHasPingListeners); + NODE_DEFINE_CONSTANT(target, kSessionHasAltsvcListeners); + // Method to fetch the nghttp2 string description of an nghttp2 error code env->SetMethod(target, "nghttp2ErrorString", HttpErrorString); diff --git a/src/node_http2.h b/src/node_http2.h index 91c0673d295910..89c0b7d38de3ea 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -675,6 +675,23 @@ class Http2Stream::Provider::Stream : public Http2Stream::Provider { void* user_data); }; +// 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 +}; + +enum SessionBitfieldFlags { + kSessionHasRemoteSettingsListeners, + kSessionRemoteSettingsIsUpToDate, + kSessionHasPingListeners, + kSessionHasAltsvcListeners +}; class Http2Session : public AsyncWrap, public StreamListener { public: @@ -961,6 +978,9 @@ class Http2Session : public AsyncWrap, public StreamListener { // The underlying nghttp2_session handle nghttp2_session* session_; + // JS-accessible numeric fields, as indexed by SessionUint8Fields. + uint8_t js_fields_[kSessionUint8FieldCount] = {}; + // The session type: client or server nghttp2_session_type session_type_;