Skip to content

Commit 0463221

Browse files
jasnellMylesBorins
authored andcommittedMay 2, 2018
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]>
1 parent ea98fd5 commit 0463221

11 files changed

+269
-164
lines changed
 

‎lib/internal/http2/core.js

+31-26
Original file line numberDiff line numberDiff line change
@@ -322,23 +322,14 @@ function onStreamRead(nread, buf, handle) {
322322

323323
// Called when the remote peer settings have been updated.
324324
// Resets the cached settings.
325-
function onSettings(ack) {
325+
function onSettings() {
326326
const session = this[kOwner];
327327
if (session.destroyed)
328328
return;
329329
session[kUpdateTimer]();
330-
let event = 'remoteSettings';
331-
if (ack) {
332-
debug(`Http2Session ${sessionName(session[kType])}: settings acknowledged`);
333-
if (session[kState].pendingAck > 0)
334-
session[kState].pendingAck--;
335-
session[kLocalSettings] = undefined;
336-
event = 'localSettings';
337-
} else {
338-
debug(`Http2Session ${sessionName(session[kType])}: new settings received`);
339-
session[kRemoteSettings] = undefined;
340-
}
341-
process.nextTick(emit, session, event, session[event]);
330+
debug(`Http2Session ${sessionName(session[kType])}: new settings received`);
331+
session[kRemoteSettings] = undefined;
332+
process.nextTick(emit, session, 'remoteSettings', session.remoteSettings);
342333
}
343334

344335
// If the stream exists, an attempt will be made to emit an event
@@ -538,15 +529,32 @@ function onSessionInternalError(code) {
538529
this[kOwner].destroy(new NghttpError(code));
539530
}
540531

532+
function settingsCallback(cb, ack, duration) {
533+
this[kState].pendingAck--;
534+
this[kLocalSettings] = undefined;
535+
if (ack) {
536+
debug(`Http2Session ${sessionName(this[kType])}: settings received`);
537+
const settings = this.localSettings;
538+
if (typeof cb === 'function')
539+
cb(null, settings, duration);
540+
this.emit('localSettings', settings);
541+
} else {
542+
debug(`Http2Session ${sessionName(this[kType])}: settings canceled`);
543+
if (typeof cb === 'function')
544+
cb(new errors.Error('ERR_HTTP2_SETTINGS_CANCEL'));
545+
}
546+
}
547+
541548
// Submits a SETTINGS frame to be sent to the remote peer.
542-
function submitSettings(settings) {
549+
function submitSettings(settings, callback) {
543550
if (this.destroyed)
544551
return;
545552
debug(`Http2Session ${sessionName(this[kType])}: submitting settings`);
546553
this[kUpdateTimer]();
547-
this[kLocalSettings] = undefined;
548554
updateSettingsBuffer(settings);
549-
this[kHandle].settings();
555+
if (!this[kHandle].settings(settingsCallback.bind(this, callback))) {
556+
this.destroy(new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK'));
557+
}
550558
}
551559

552560
// Submits a PRIORITY frame to be sent to the remote peer
@@ -781,7 +789,6 @@ class Http2Session extends EventEmitter {
781789
streams: new Map(),
782790
pendingStreams: new Set(),
783791
pendingAck: 0,
784-
maxPendingAck: Math.max(1, (options.maxPendingAck | 0) || 10),
785792
writeQueueSize: 0
786793
};
787794

@@ -948,21 +955,19 @@ class Http2Session extends EventEmitter {
948955
}
949956

950957
// Submits a SETTINGS frame to be sent to the remote peer.
951-
settings(settings) {
958+
settings(settings, callback) {
952959
if (this.destroyed)
953960
throw new errors.Error('ERR_HTTP2_INVALID_SESSION');
954-
955961
assertIsObject(settings, 'settings');
956962
settings = validateSettings(settings);
957-
const state = this[kState];
958-
if (state.pendingAck === state.maxPendingAck) {
959-
throw new errors.Error('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
960-
this[kState].pendingAck);
961-
}
963+
964+
if (callback && typeof callback !== 'function')
965+
throw new errors.TypeError('ERR_INVALID_CALLBACK');
962966
debug(`Http2Session ${sessionName(this[kType])}: sending settings`);
963967

964-
state.pendingAck++;
965-
const settingsFn = submitSettings.bind(this, settings);
968+
this[kState].pendingAck++;
969+
970+
const settingsFn = submitSettings.bind(this, settings, callback);
966971
if (this.connecting) {
967972
this.once('connect', settingsFn);
968973
return;

‎lib/internal/http2/util.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,8 @@ const IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS = 3;
174174
const IDX_OPTIONS_PADDING_STRATEGY = 4;
175175
const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
176176
const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6;
177-
const IDX_OPTIONS_FLAGS = 7;
177+
const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7;
178+
const IDX_OPTIONS_FLAGS = 8;
178179

179180
function updateOptionsBuffer(options) {
180181
var flags = 0;
@@ -213,6 +214,11 @@ function updateOptionsBuffer(options) {
213214
optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS] =
214215
options.maxOutstandingPings;
215216
}
217+
if (typeof options.maxOutstandingSettings === 'number') {
218+
flags |= (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS);
219+
optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS] =
220+
Math.max(1, options.maxOutstandingSettings);
221+
}
216222
optionsBuffer[IDX_OPTIONS_FLAGS] = flags;
217223
}
218224

‎src/async_wrap.h

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ namespace node {
4444
V(HTTP2SESSION) \
4545
V(HTTP2STREAM) \
4646
V(HTTP2PING) \
47+
V(HTTP2SETTINGS) \
4748
V(HTTPPARSER) \
4849
V(JSSTREAM) \
4950
V(PIPECONNECTWRAP) \

‎src/env.h

+1
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ class ModuleWrap;
317317
V(domains_stack_array, v8::Array) \
318318
V(http2ping_constructor_template, v8::ObjectTemplate) \
319319
V(http2stream_constructor_template, v8::ObjectTemplate) \
320+
V(http2settings_constructor_template, v8::ObjectTemplate) \
320321
V(inspector_console_api_object, v8::Object) \
321322
V(module_load_list_array, v8::Array) \
322323
V(pbkdf2_constructor_template, v8::ObjectTemplate) \

0 commit comments

Comments
 (0)
Please sign in to comment.