Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
async_hooks,http: set HTTPParser trigger to socket
This allows more easy tracking of where HTTP requests come from. Before
this change the HTTPParser would have the HTTPServer as the
triggerAsyncId.

The HTTPParser will still have the executionAsyncId set to the HTTP
Server so that information is still directly available. Indirectly, the
TCP socket itself also has its triggerAsyncId set to the TCP Server.
  • Loading branch information
AndreasMadsen committed Jan 10, 2018
commit 8e5f1a705e39810ddb8dfbc95fa4fd82007e05a2
28 changes: 19 additions & 9 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ const {
} = require('_http_common');
const { OutgoingMessage } = require('_http_outgoing');
const { outHeadersKey, ondrain } = require('internal/http');
const {
defaultTriggerAsyncIdScope,
getOrSetAsyncId
} = require('internal/async_hooks');
const errors = require('internal/errors');
const Buffer = require('buffer').Buffer;

Expand Down Expand Up @@ -292,20 +296,26 @@ Server.prototype.setTimeout = function setTimeout(msecs, callback) {


function connectionListener(socket) {
defaultTriggerAsyncIdScope(
getOrSetAsyncId(socket), [this, socket], connectionListenerInternal
);
}

function connectionListenerInternal(server, socket) {
debug('SERVER new http connection');

httpSocketSetup(socket);

// Ensure that the server property of the socket is correctly set.
// See https://github.com/nodejs/node/issues/13435
if (socket.server === null)
socket.server = this;
socket.server = server;

// If the user has added a listener to the server,
// request, or response, then it's their responsibility.
// otherwise, destroy on timeout by default
if (this.timeout && typeof socket.setTimeout === 'function')
socket.setTimeout(this.timeout);
if (server.timeout && typeof socket.setTimeout === 'function')
socket.setTimeout(server.timeout);
socket.on('timeout', socketOnTimeout);

var parser = parsers.alloc();
Expand All @@ -315,8 +325,8 @@ function connectionListener(socket) {
parser.incoming = null;

// Propagate headers limit from server instance to parser
if (typeof this.maxHeadersCount === 'number') {
parser.maxHeaderPairs = this.maxHeadersCount << 1;
if (typeof server.maxHeadersCount === 'number') {
parser.maxHeaderPairs = server.maxHeadersCount << 1;
} else {
// Set default value because parser may be reused from FreeList
parser.maxHeaderPairs = 2000;
Expand All @@ -336,16 +346,16 @@ function connectionListener(socket) {
outgoingData: 0,
keepAliveTimeoutSet: false
};
state.onData = socketOnData.bind(undefined, this, socket, parser, state);
state.onEnd = socketOnEnd.bind(undefined, this, socket, parser, state);
state.onData = socketOnData.bind(undefined, server, socket, parser, state);
state.onEnd = socketOnEnd.bind(undefined, server, socket, parser, state);
state.onClose = socketOnClose.bind(undefined, socket, state);
state.onDrain = socketOnDrain.bind(undefined, socket, state);
socket.on('data', state.onData);
socket.on('error', socketOnError);
socket.on('end', state.onEnd);
socket.on('close', state.onClose);
socket.on('drain', state.onDrain);
parser.onIncoming = parserOnIncoming.bind(undefined, this, socket, state);
parser.onIncoming = parserOnIncoming.bind(undefined, server, socket, state);

// We are consuming socket, so it won't get any actual data
socket.on('resume', onSocketResume);
Expand All @@ -364,7 +374,7 @@ function connectionListener(socket) {
}
}
parser[kOnExecute] =
onParserExecute.bind(undefined, this, socket, parser, state);
onParserExecute.bind(undefined, server, socket, parser, state);

socket._paused = false;
}
Expand Down
12 changes: 11 additions & 1 deletion lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const async_wrap = process.binding('async_wrap');
* It has a fixed size, so if that is exceeded, calls to the native
* side are used instead in pushAsyncIds() and popAsyncIds().
*/
const { async_hook_fields, async_id_fields } = async_wrap;
const { async_id_symbol, async_hook_fields, async_id_fields } = async_wrap;
// Store the pair executionAsyncId and triggerAsyncId in a std::stack on
// Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the
// current execution stack. This is unwound as each resource exits. In the case
Expand Down Expand Up @@ -248,6 +248,15 @@ function newUid() {
return ++async_id_fields[kAsyncIdCounter];
}

function getOrSetAsyncId(object) {
if (object.hasOwnProperty(async_id_symbol)) {
return object[async_id_symbol];
}

return object[async_id_symbol] = newUid();
}


// Return the triggerAsyncId meant for the constructor calling it. It's up to
// the user to safeguard this call and make sure it's zero'd out when the
// constructor is complete.
Expand Down Expand Up @@ -378,6 +387,7 @@ module.exports = {
disableHooks,
// Internal Embedder API
newUid,
getOrSetAsyncId,
getDefaultTriggerAsyncId,
defaultTriggerAsyncIdScope,
emitInit: emitInitScript,
Expand Down
52 changes: 52 additions & 0 deletions test/async-hooks/test-graph.http.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
'use strict';

const common = require('../common');
const initHooks = require('./init-hooks');
const verifyGraph = require('./verify-graph');
const http = require('http');

const hooks = initHooks();
hooks.enable();

const server = http.createServer(common.mustCall(function(req, res) {
res.end();
this.close(common.mustCall());
}));
server.listen(0, common.mustCall());

http.get(`http://127.0.0.1:${server.address().port}`, common.mustCall());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be moved inside the listening event handler? Wonder if this could possibly end up being flaky


process.on('exit', function() {
hooks.disable();

verifyGraph(
hooks,
[ { type: 'TCPSERVERWRAP',
id: 'tcpserver:1',
triggerAsyncId: null },
{ type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: null },
{ type: 'TCPCONNECTWRAP',
id: 'tcpconnect:1',
triggerAsyncId: 'tcp:1' },
{ type: 'HTTPPARSER', id: 'httpparser:1', triggerAsyncId: null },
{ type: 'HTTPPARSER', id: 'httpparser:2', triggerAsyncId: null },
{ type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' },
{ type: 'Timeout', id: 'timeout:1', triggerAsyncId: 'tcp:2' },
{ type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: 'tcp:2' },
{ type: 'HTTPPARSER',
id: 'httpparser:3',
triggerAsyncId: 'tcp:2' },
{ type: 'HTTPPARSER',
id: 'httpparser:4',
triggerAsyncId: 'tcp:2' },
{ type: 'Timeout',
id: 'timeout:2',
triggerAsyncId: 'httpparser:4' },
{ type: 'TIMERWRAP',
id: 'timer:2',
triggerAsyncId: 'httpparser:4' },
{ type: 'SHUTDOWNWRAP',
id: 'shutdown:1',
triggerAsyncId: 'tcp:2' } ]
);
});