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

No closures #23

Closed
wants to merge 3 commits into from
Closed

No closures #23

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99a32a9
more perf updates
mcollina Dec 5, 2016
16e0c23
Removed most closures.
mcollina Dec 5, 2016
839a3f2
From Map() to basic {}.
mcollina Dec 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 74 additions & 76 deletions lib/internal/http2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,99 +312,79 @@ class Http2Stream extends Duplex {
if (this._handle) {
this._handle.setLocalWindowSize(size);
} else {
this.once('handle', () => {
this._handle.setLocalWindowSize(size);
});
this.once('handle', this.setLocalWindowSize.bind(this, size));
}
}

changeStreamPriority(parentId, priority, exclusive) {
if (this._handle) {
this._handle.changeStreamPriority(parentId, priority, exclusive);
} else {
this.once('handle', () => {
this._handle.changeStreamPriority(parentId, priority, exclusive);
});
this.once('handle', this.changeStreamPriority.bind(this, parentId, priority, exclusive));
}
}

respond() {
if (this._handle) {
this._handle.respond();
} else {
this.once('handle', () => {
this._handle.respond();
});
this.once('handle', onHandleRespond);
}
}

resume() {
if (this._handle) {
this._handle.resume();
} else {
this.once('handle', () => {
this._handle.resume();
});
this.once('handle', onHandleResume);
}
}

sendContinue() {
if (this._handle) {
this._handle.sendContinue();
} else {
this.once('handle', () => {
this._handle.sendContinue();
});
this.once('handle', this.sendContinue.bind(this));
}
}

sendPriority(parentId, priority, exclusive) {
if (this._handle) {
this._handle.sendPriority(parentId, priority, exclusive);
} else {
this.once('handle', () => {
this._handle.sendPriority(parentId, priority, exclusive);
});
this.once('handle', this.sendPriority.bind(this, parentId, priority, exclusive));
}
}

sendRstStream(code) {
if (this._handle) {
this._handle.sendRstStream(code);
} else {
this.once('handle', () => {
this._handle.sendRstStream(code);
});
this.once('handle', this.sendRstStream.bind(this, code));
}
}

sendPushPromise(headers) {
if (this._handle) {
return this._handle.sendPushPromise(mapToHeaders(headers));
} else {
this.once('handle', () => {
this._handle.sendPushPromise(mapToHeaders(headers));
});
this.once('handle', this.sendPushPromise.bind(this, headers));
}
}

addHeader(name, value, noindex) {
if (this._handle) {
this._handle.addHeader(name, value, noindex);
} else {
this.once('handle', () => {
this._handle.addHeader(name, value, noindex);
});
this.once('handle', this.addHeader.bind(this, name, value, noindex));
}
}

addTrailer(name, value, noindex) {
if (this._handle) {
this._handle.addTrailer(name, value, noindex);
} else {
this.once('handle', () => {
this._handle.addTrailer(name, value, noindex);
});
this.once('handle', this.addTrailer.bind(this, name, value, noindex));
}
}

Expand Down Expand Up @@ -464,29 +444,41 @@ class Http2Stream extends Duplex {
if (this._handle) {
this._handle.finishedWriting();
} else {
this.on('handle', () => {
this._handle.finishedWriting();
});
this.on('handle', onHandleFinishedWriting);
}
}

_read(n) {
if (this._handle) {
this._handle.readStart();
} else {
this.once('handle', () => {
this._handle.readStart();
});
this.once('handle', onHandleReadStart);
}
}
}

function onHandleReadStart() {
this._handle.readStart();
}

function onHandleFinishedWriting() {
this._handle.finishedWriting();
}

function onHandleWrite(data, encoding, cb) {
return function onWriteFinished() {
this._write(data, encoding, cb);
};
}

function onHandleRespond() {
this._handle.respond();
}

function onHandleResume() {
this._handle.resume();
}

class Http2Session extends EventEmitter {
constructor(type, options, socket) {
super();
Expand All @@ -495,6 +487,7 @@ class Http2Session extends EventEmitter {
this[kHandle] = sessions.alloc();
this[kHandle][kOwner] = this;
this[kHandle].reinitialize(type, options, socket._handle._externalStream);
this[kSocket] = socket;
}

reset() {
Expand Down Expand Up @@ -1103,28 +1096,29 @@ function sessionOnStreamClose(stream, code) {
response[kStream] = undefined;
stream[kRequest] = undefined;
stream[kResponse] = undefined;
setImmediate(() => maybeDestroyStream(stream));
setImmediate(maybeDestroyStream, stream);
}

function sessionOnError(server, socket) {
function fn(error) {
if (server.listenerCount('sessionError') > 0) {
server.emit('sessionError', error);
return;
}
socket.destroy(error);
function sessionOnError() {
const session = this;
const server = session[kServer];
const socket = session[kSocket];

if (server.listenerCount('sessionError') > 0) {
server.emit('sessionError', error);
return;
}
return fn;
socket.destroy(error);
}

function socketOnTimeout(server, session) {
function fn() {
if (!server.emit('timeout', this)) {
// Session timed out, attempt a graceful exit
session.gracefulTerminate(() => this.destroy());
}
function socketOnTimeout() {
const socket = this;
const server = socket[kServer];

if (!server.emit('timeout', this)) {
// Session timed out, attempt a graceful exit
session.gracefulTerminate(this.destroy.bind(this));
}
return fn;
}

function socketOnceError(error) {
Expand Down Expand Up @@ -1248,7 +1242,7 @@ function connectionListener(socket) {
session[kServer] = this;
socket[kServer] = this;

session.on('error', sessionOnError(this, socket));
session.on('error', sessionOnError);

// Disable TLS Negotiation on this socket. The HTTP/2 allows renegotiation to
// happen up until the initial HTTP/2 session bootstrap. After that, it is
Expand All @@ -1259,7 +1253,7 @@ function connectionListener(socket) {
// Set up the timeout listener
if (this.timeout)
socket.setTimeout(this.timeout);
socket.on('timeout', socketOnTimeout(this, session));
socket.on('timeout', socketOnTimeout);

// Destroy the session if the socket is destroyed
const destroySocket = socket.destroy;
Expand Down Expand Up @@ -1307,17 +1301,19 @@ function initializeTLSOptions(options) {
return options;
}

function onErrorSecureServerSession(err, conn) {
if (!this.emit('clientError', err, conn))
conn.destroy(err);
}

class Http2SecureServerSession extends TLSServer {
constructor(options, requestListener) {
super(initializeTLSOptions(options), connectionListener);
this[kOptions] = options;
this.timeout = kDefaultSocketTimeout;
if (typeof requestListener === 'function')
this.on('request', requestListener);
this.on('tlsClientError', (err, conn) => {
if (!this.emit('clientError', err, conn))
conn.destroy(err);
});
this.on('tlsClientError', onErrorSecureServerSession);
}

setTimeout(msecs, callback) {
Expand Down Expand Up @@ -1440,7 +1436,7 @@ function clientSessionOnStreamClose(stream, code) {
response[kStream] = undefined;
stream[kRequest] = undefined;
stream[kResponse] = undefined;
setImmediate(() => maybeDestroyStream(stream));
setImmediate(maybeDestroyStream, stream);
}

function initializeClientOptions(options) {
Expand Down Expand Up @@ -1475,9 +1471,8 @@ class Http2ClientSession extends EventEmitter {
this[kSocket] = socket;

const session = this[kSession] = createClientSession(options, socket);
socket.once('error', (error) => {
console.log(error);
});
// TODO remove this
socket.once('error', console.log);
socket.on('resume', socketOnResume);
socket.on('pause', socketOnPause);
socket.on('drain', socketOnDrain);
Expand Down Expand Up @@ -1605,20 +1600,7 @@ class Http2ClientRequest extends Http2Outgoing {
const _handle = this.stream.session.request(mapToHeaders(this[kHeaders]), true);
if (_handle instanceof http2.Http2Stream) {
this[kId] = _handle.getId();
this.stream.once('handle', () => {
if (this[kTrailers] instanceof Map) {
for (const v of this[kTrailers]) {
const key = String(v[0]);
const value = v[1];
if (Array.isArray(value) && value.length > 0) {
for (const item of value)
this.stream.addTrailer(key, String(item));
} else {
this.stream.addTrailer(key, String(value));
}
}
}
});
this.stream.once('handle', addTrailers)
this.stream._handle = _handle;
}
}
Expand All @@ -1629,6 +1611,22 @@ class Http2ClientRequest extends Http2Outgoing {
}
}

function addTrailers () {
const request = this[kRequest];
if (request[kTrailers] instanceof Map) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use the internal util for this? Is there a chance that the Map was created in a different context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evanlucas ... no, the Map here should always be created in the same context. We could likely just have this be a truthy check to be honest.

for (const v of request[kTrailers]) {
const key = String(v[0]);
const value = v[1];
if (Array.isArray(value) && value.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't for of deopt for arrays?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... I believe it does... (or at least did... I don't know if it still does)

for (const item of value)
this.addTrailer(key, String(item));
} else {
this.addTrailer(key, String(value));
}
}
}
}

class Http2ClientResponse extends Http2Incoming {
constructor(stream, headers, options) {
super(stream, headers, options);
Expand Down