Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AsyncWrap: HTTP has no handle context #3241

Closed
AndreasMadsen opened this issue Oct 7, 2015 · 5 comments
Closed

AsyncWrap: HTTP has no handle context #3241

AndreasMadsen opened this issue Oct 7, 2015 · 5 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. http Issues or PRs related to the http subsystem. question Issues that look for answers.

Comments

@AndreasMadsen
Copy link
Member

When creating a simple HTTPserver and connecting to it one looses the handle context. Meaning there is no way to know what handle or callback created the new handle.

'use strict';
const http = require('http');
const server = http.createServer(function (req, res) {
  res.end('hallo world');
});

server.listen(0, 'localhost', function () {
  const addr = server.address();
  const req = http.get(`http://${addr.address}:${addr.port}`, function (res) {
    res.resume();
    res.once('end', server.close.bind(server));
  });
});

Note that this issues exists even after applying #3216 (adds parent to init hook)

complete test case: https://gist.github.com/AndreasMadsen/f56bbdbcd18a2c6358f3
dprof dump: http://bl.ocks.org/AndreasMadsen/raw/6c460eb0e7d6eeadb31a/

skaermbillede 2015-10-07 kl 20 21 09

/cc @trevnorris

@mscdex mscdex added c++ Issues and PRs that require attention from people who are familiar with C++. http Issues or PRs related to the http subsystem. labels Oct 7, 2015
@trevnorris
Copy link
Contributor

Unfortunately getting around this requires knowing implementation details, but here's an example:

'use strict';
const http = require('http');
const async_wrap = process.binding('async_wrap');
const print = process._rawDebug;
let client;

function init(p, parent) {
  if (parent) client = this;
}

function noop() { }

async_wrap.setupHooks(init, noop, noop);
async_wrap.enable();


const server = http.createServer(function(req, res) {
  // Print if this client matches the init w/ parent.
  print(req.client._handle === client);
  res.end('hello world');
});


server.listen(0, 'localhost', function() {
  // Disable listening to simply test.
  async_wrap.disable();
  http.get('http://localhost:' + server.address().port + '/', (res) => {
    res.resume();
    res.once('end', () => server.close());
  });
});

This demonstrates that the client handle for a given http request can still be retrieved. Though this is not ideal. I'll look into propagating this information during the HTTPParser phase.

One issue with your test is that it will exit on cases like SHUTDOWNWRAP. May want to focus your test using .disable() like I have above.

@trevnorris
Copy link
Contributor

It should work the same as TCP if you listened for the http server's 'connection' event. Instead of the default 'request' event.

@AndreasMadsen
Copy link
Member Author

One issue with your test is that it will exit on cases like SHUTDOWNWRAP. May want to focus your test using .disable() like I have above.

Sorry I don't follow you.

  1. I don't understand your SHUTDOWNWRAP reference, could you provide an example.

  2. I find that the first part of the test (https://gist.github.com/AndreasMadsen/f56bbdbcd18a2c6358f3#file-test_http-js-L7L52) i fairly generic. It just checks that there always is a callback or handle context for the new handle. I think that should always be the case, otherwise making a long-stack-trace tool would not be possible.

It should work the same as TCP if you listened for the http server's 'connection' event. Instead of the default 'request' event.

I think we are on the same page here, but just to be sure. One of my goals is to write a long-stack-trace tool, that doesn't require any extra interaction from the user except adding -r trace. Changing the the eventlistener from request to connection and feeding back the socket handle to the server handle, definitely qualifies as extra interaction.

@AndreasMadsen
Copy link
Member Author

Closed by #5419

@trevnorris
Copy link
Contributor

may need to reopen unless I can fix a regression the patch introduced...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. http Issues or PRs related to the http subsystem. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

4 participants