http2: convert Http2Settings to an AsyncWrap

Backport-PR-URL: #18050 Backport-PR-URL: #20456 PR-URL: #17763 Refs: #17746 Reviewed-By: Matteo Collina <[email protected]>
nodejs · May 2, 2018 · 0463221 · 0463221
1 parent ea98fd5
commit 0463221
Show file tree

Hide file tree

Showing 11 changed files with 269 additions and 164 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -322,23 +322,14 @@ function onStreamRead(nread, buf, handle) {
 
 // Called when the remote peer settings have been updated.
 // Resets the cached settings.
-function onSettings(ack) {
+function onSettings() {
   const session = this[kOwner];
   if (session.destroyed)
     return;
   session[kUpdateTimer]();
-  let event = 'remoteSettings';
-  if (ack) {
-    debug(`Http2Session ${sessionName(session[kType])}: settings acknowledged`);
-    if (session[kState].pendingAck > 0)
-      session[kState].pendingAck--;
-    session[kLocalSettings] = undefined;
-    event = 'localSettings';
-  } else {
-    debug(`Http2Session ${sessionName(session[kType])}: new settings received`);
-    session[kRemoteSettings] = undefined;
-  }
-  process.nextTick(emit, session, event, session[event]);
+  debug(`Http2Session ${sessionName(session[kType])}: new settings received`);
+  session[kRemoteSettings] = undefined;
+  process.nextTick(emit, session, 'remoteSettings', session.remoteSettings);
 }
 
 // If the stream exists, an attempt will be made to emit an event
@@ -538,15 +529,32 @@ function onSessionInternalError(code) {
     this[kOwner].destroy(new NghttpError(code));
 }
 
+function settingsCallback(cb, ack, duration) {
+  this[kState].pendingAck--;
+  this[kLocalSettings] = undefined;
+  if (ack) {
+    debug(`Http2Session ${sessionName(this[kType])}: settings received`);
+    const settings = this.localSettings;
+    if (typeof cb === 'function')
+      cb(null, settings, duration);
+    this.emit('localSettings', settings);
+  } else {
+    debug(`Http2Session ${sessionName(this[kType])}: settings canceled`);
+    if (typeof cb === 'function')
+      cb(new errors.Error('ERR_HTTP2_SETTINGS_CANCEL'));
+  }
+}
+
 // Submits a SETTINGS frame to be sent to the remote peer.
-function submitSettings(settings) {
+function submitSettings(settings, callback) {
   if (this.destroyed)
     return;
   debug(`Http2Session ${sessionName(this[kType])}: submitting settings`);
   this[kUpdateTimer]();
-  this[kLocalSettings] = undefined;
   updateSettingsBuffer(settings);
-  this[kHandle].settings();
+  if (!this[kHandle].settings(settingsCallback.bind(this, callback))) {
+    this.destroy(new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK'));
+  }
 }
 
 // Submits a PRIORITY frame to be sent to the remote peer
@@ -781,7 +789,6 @@ class Http2Session extends EventEmitter {
       streams: new Map(),
       pendingStreams: new Set(),
       pendingAck: 0,
-      maxPendingAck: Math.max(1, (options.maxPendingAck | 0) || 10),
       writeQueueSize: 0
     };
 
@@ -948,21 +955,19 @@ class Http2Session extends EventEmitter {
   }
 
   // Submits a SETTINGS frame to be sent to the remote peer.
-  settings(settings) {
+  settings(settings, callback) {
     if (this.destroyed)
       throw new errors.Error('ERR_HTTP2_INVALID_SESSION');
-
     assertIsObject(settings, 'settings');
     settings = validateSettings(settings);
-    const state = this[kState];
-    if (state.pendingAck === state.maxPendingAck) {
-      throw new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
-                             this[kState].pendingAck);
-    }
+
+    if (callback && typeof callback !== 'function')
+      throw new errors.TypeError('ERR_INVALID_CALLBACK');
     debug(`Http2Session ${sessionName(this[kType])}: sending settings`);
 
-    state.pendingAck++;
-    const settingsFn = submitSettings.bind(this, settings);
+    this[kState].pendingAck++;
+
+    const settingsFn = submitSettings.bind(this, settings, callback);
     if (this.connecting) {
       this.once('connect', settingsFn);
       return;

diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
@@ -174,7 +174,8 @@ const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3;
 const IDX_OPTIONS_PADDING_STRATEGY = 4;
 const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
 const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6;
-const IDX_OPTIONS_FLAGS = 7;
+const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7;
+const IDX_OPTIONS_FLAGS = 8;
 
 function updateOptionsBuffer(options) {
   var flags = 0;
@@ -213,6 +214,11 @@ function updateOptionsBuffer(options) {
     optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS] =
       options.maxOutstandingPings;
   }
+  if (typeof options.maxOutstandingSettings === 'number') {
+    flags |= (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS);
+    optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS] =
+      Math.max(1, options.maxOutstandingSettings);
+  }
   optionsBuffer[IDX_OPTIONS_FLAGS] = flags;
 }
 

diff --git a/src/async_wrap.h b/src/async_wrap.h
@@ -44,6 +44,7 @@ namespace node {
   V(HTTP2SESSION)                                                             \
   V(HTTP2STREAM)                                                              \
   V(HTTP2PING)                                                                \
+  V(HTTP2SETTINGS)                                                            \
   V(HTTPPARSER)                                                               \
   V(JSSTREAM)                                                                 \
   V(PIPECONNECTWRAP)                                                          \

diff --git a/src/env.h b/src/env.h
@@ -317,6 +317,7 @@ class ModuleWrap;
   V(domains_stack_array, v8::Array)                                           \
   V(http2ping_constructor_template, v8::ObjectTemplate)                       \
   V(http2stream_constructor_template, v8::ObjectTemplate)                     \
+  V(http2settings_constructor_template, v8::ObjectTemplate)                   \
   V(inspector_console_api_object, v8::Object)                                 \
   V(module_load_list_array, v8::Array)                                        \
   V(pbkdf2_constructor_template, v8::ObjectTemplate)                          \