diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 99b996a1af6bc8..3e28aa21a8c3c3 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -236,10 +236,10 @@ The `type` is a string identifying the type of resource that caused resource's constructor. ```text -FSEVENTWRAP, FSREQCALLBACK, GETADDRINFOREQWRAP, GETNAMEINFOREQWRAP, HTTPPARSER, -JSSTREAM, PIPECONNECTWRAP, PIPEWRAP, PROCESSWRAP, QUERYWRAP, SHUTDOWNWRAP, -SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPSERVERWRAP, TCPWRAP, TTYWRAP, -UDPSENDWRAP, UDPWRAP, WRITEWRAP, ZLIB, SSLCONNECTION, PBKDF2REQUEST, +FSEVENTWRAP, FSREQCALLBACK, GETADDRINFOREQWRAP, GETNAMEINFOREQWRAP, HTTPINCOMINGMESSAGE, +HTTPCLIENTREQUEST, JSSTREAM, PIPECONNECTWRAP, PIPEWRAP, PROCESSWRAP, QUERYWRAP, +SHUTDOWNWRAP, SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPSERVERWRAP, TCPWRAP, +TTYWRAP, UDPSENDWRAP, UDPWRAP, WRITEWRAP, ZLIB, SSLCONNECTION, PBKDF2REQUEST, RANDOMBYTESREQUEST, TLSWRAP, Microtask, Timeout, Immediate, TickObject ``` diff --git a/lib/_http_client.js b/lib/_http_client.js index e23c4909ddf3d0..895e0bd6009dae 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -632,11 +632,10 @@ function emitFreeNT(socket) { } function tickOnSocket(req, socket) { - const isParserReused = parsers.hasItems(); const parser = parsers.alloc(); req.socket = socket; req.connection = socket; - parser.reinitialize(HTTPParser.RESPONSE, isParserReused); + parser.initialize(HTTPParser.RESPONSE, req); parser.socket = socket; parser.outgoing = req; req.parser = parser; diff --git a/lib/_http_server.js b/lib/_http_server.js index ff29b9d2b60262..9f62872e8ec5e8 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -126,6 +126,12 @@ const STATUS_CODES = { const kOnExecute = HTTPParser.kOnExecute | 0; +class HTTPServerAsyncResource { + constructor(type, socket) { + this.type = type; + this.socket = socket; + } +} function ServerResponse(req) { OutgoingMessage.call(this); @@ -349,9 +355,15 @@ function connectionListenerInternal(server, socket) { socket.setTimeout(server.timeout); socket.on('timeout', socketOnTimeout); - const isParserReused = parsers.hasItems(); const parser = parsers.alloc(); - parser.reinitialize(HTTPParser.REQUEST, isParserReused); + + // TODO(addaleax): This doesn't play well with the + // `async_hooks.currentResource()` proposal, see + // https://github.com/nodejs/node/pull/21313 + parser.initialize( + HTTPParser.REQUEST, + new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket) + ); parser.socket = socket; // We are starting to wait for our headers. diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h index 6ef968933b8c1f..53972493052f5e 100644 --- a/src/async_wrap-inl.h +++ b/src/async_wrap-inl.h @@ -34,6 +34,11 @@ inline AsyncWrap::ProviderType AsyncWrap::provider_type() const { return provider_type_; } +inline AsyncWrap::ProviderType AsyncWrap::set_provider_type( + AsyncWrap::ProviderType provider) { + provider_type_ = provider; + return provider_type_; +} inline double AsyncWrap::get_async_id() const { return async_id_; diff --git a/src/async_wrap.cc b/src/async_wrap.cc index dd84f992273a27..9cfa0924c79568 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -602,11 +602,15 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) { env->destroy_async_id_list()->push_back(async_id); } +void AsyncWrap::AsyncReset(double execution_async_id, bool silent) { + AsyncReset(object(), execution_async_id, silent); +} // Generalized call for both the constructor and for handles that are pooled // and reused over their lifetime. This way a new uid can be assigned when // the resource is pulled out of the pool and put back into use. -void AsyncWrap::AsyncReset(double execution_async_id, bool silent) { +void AsyncWrap::AsyncReset(Local resource, double execution_async_id, + bool silent) { if (async_id_ != -1) { // This instance was in use before, we have already emitted an init with // its previous async_id and need to emit a matching destroy for that @@ -643,7 +647,7 @@ void AsyncWrap::AsyncReset(double execution_async_id, bool silent) { if (silent) return; - EmitAsyncInit(env(), object(), + EmitAsyncInit(env(), resource, env()->async_hooks()->provider_string(provider_type()), async_id_, trigger_async_id_); } diff --git a/src/async_wrap.h b/src/async_wrap.h index d84f0c6d2f61db..bcd37bb0c0d5b6 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -46,7 +46,8 @@ namespace node { V(HTTP2STREAM) \ V(HTTP2PING) \ V(HTTP2SETTINGS) \ - V(HTTPPARSER) \ + V(HTTPINCOMINGMESSAGE) \ + V(HTTPCLIENTREQUEST) \ V(JSSTREAM) \ V(MESSAGEPORT) \ V(PIPECONNECTWRAP) \ @@ -147,11 +148,16 @@ class AsyncWrap : public BaseObject { static void DestroyAsyncIdsCallback(Environment* env, void* data); inline ProviderType provider_type() const; + inline ProviderType set_provider_type(ProviderType provider); inline double get_async_id() const; inline double get_trigger_async_id() const; + void AsyncReset(v8::Local resource, + double execution_async_id = -1, + bool silent = false); + void AsyncReset(double execution_async_id = -1, bool silent = false); // Only call these within a valid HandleScope. @@ -202,7 +208,7 @@ class AsyncWrap : public BaseObject { ProviderType provider, double execution_async_id, bool silent); - const ProviderType provider_type_; + ProviderType provider_type_; // Because the values may be Reset(), cannot be made const. double async_id_ = -1; double trigger_async_id_; diff --git a/src/node_http_parser_impl.h b/src/node_http_parser_impl.h index 75ea992dda9748..a8a8db554783fb 100644 --- a/src/node_http_parser_impl.h +++ b/src/node_http_parser_impl.h @@ -152,11 +152,13 @@ struct StringPtr { size_t size_; }; - class Parser : public AsyncWrap, public StreamListener { public: Parser(Environment* env, Local wrap, parser_type_t type) - : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTPPARSER), + : AsyncWrap(env, wrap, + type == HTTP_REQUEST ? + AsyncWrap::PROVIDER_HTTPINCOMINGMESSAGE : + AsyncWrap::PROVIDER_HTTPCLIENTREQUEST), current_buffer_len_(0), current_buffer_data_(nullptr) { Init(type); @@ -503,12 +505,12 @@ class Parser : public AsyncWrap, public StreamListener { } - static void Reinitialize(const FunctionCallbackInfo& args) { + static void Initialize(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args[0]->IsInt32()); - CHECK(args[1]->IsBoolean()); - bool isReused = args[1]->IsTrue(); + CHECK(args[1]->IsObject()); + parser_type_t type = static_cast(args[0].As()->Value()); @@ -517,16 +519,16 @@ class Parser : public AsyncWrap, public StreamListener { ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder()); // Should always be called from the same context. CHECK_EQ(env, parser->env()); - // This parser has either just been created or it is being reused. - // We must only call AsyncReset for the latter case, because AsyncReset has - // already been called via the constructor for the former case. - if (isReused) { - parser->AsyncReset(); - } + + AsyncWrap::ProviderType provider = + (type == HTTP_REQUEST ? + AsyncWrap::PROVIDER_HTTPINCOMINGMESSAGE + : AsyncWrap::PROVIDER_HTTPCLIENTREQUEST); + + parser->set_provider_type(provider); parser->Init(type); } - template static void Pause(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -958,7 +960,7 @@ void InitializeHttpParser(Local target, env->SetProtoMethod(t, "free", Parser::Free); env->SetProtoMethod(t, "execute", Parser::Execute); env->SetProtoMethod(t, "finish", Parser::Finish); - env->SetProtoMethod(t, "reinitialize", Parser::Reinitialize); + env->SetProtoMethod(t, "initialize", Parser::Initialize); env->SetProtoMethod(t, "pause", Parser::Pause); env->SetProtoMethod(t, "resume", Parser::Pause); env->SetProtoMethod(t, "consume", Parser::Consume); diff --git a/test/async-hooks/test-httparser-reuse.js b/test/async-hooks/test-httparser-reuse.js new file mode 100644 index 00000000000000..b6d82d7d5e9087 --- /dev/null +++ b/test/async-hooks/test-httparser-reuse.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); +const http = require('http'); +const assert = require('assert'); +const { createHook } = require('async_hooks'); +const reused = Symbol('reused'); + +let reusedHTTPParser = false; +const asyncHook = createHook({ + init(asyncId, type, triggerAsyncId, resource) { + if (resource[reused]) { + reusedHTTPParser = true; + } + resource[reused] = true; + } +}); +asyncHook.enable(); + +const server = http.createServer(function(req, res) { + res.end(); +}); + +const PORT = 3000; +const url = 'http://127.0.0.1:' + PORT; + +server.listen(PORT, common.mustCall(() => { + http.get(url, common.mustCall(() => { + server.close(common.mustCall(() => { + server.listen(PORT, common.mustCall(() => { + http.get(url, common.mustCall(() => { + server.close(common.mustCall(() => { + assert.strictEqual(reusedHTTPParser, false); + })); + })); + })); + })); + })); +})); diff --git a/test/async-hooks/test-httpparser.request.js b/test/async-hooks/test-httpparser.request.js index 2e62e6eaa9842b..7ba8feefe34d5d 100644 --- a/test/async-hooks/test-httpparser.request.js +++ b/test/async-hooks/test-httpparser.request.js @@ -21,7 +21,7 @@ const request = Buffer.from( ); const parser = new HTTPParser(REQUEST); -const as = hooks.activitiesOfTypes('HTTPPARSER'); +const as = hooks.activitiesOfTypes('HTTPINCOMINGMESSAGE'); const httpparser = as[0]; assert.strictEqual(as.length, 1); @@ -47,7 +47,7 @@ process.on('exit', onexit); function onexit() { hooks.disable(); - hooks.sanityCheck('HTTPPARSER'); + hooks.sanityCheck('HTTPINCOMINGMESSAGE'); checkInvocations(httpparser, { init: 1, before: 1, after: 1, destroy: 1 }, 'when process exits'); } diff --git a/test/async-hooks/test-httpparser.response.js b/test/async-hooks/test-httpparser.response.js index 5933b61b79fd99..85d6f76525d49b 100644 --- a/test/async-hooks/test-httpparser.response.js +++ b/test/async-hooks/test-httpparser.response.js @@ -26,7 +26,7 @@ const request = Buffer.from( ); const parser = new HTTPParser(RESPONSE); -const as = hooks.activitiesOfTypes('HTTPPARSER'); +const as = hooks.activitiesOfTypes('HTTPCLIENTREQUEST'); const httpparser = as[0]; assert.strictEqual(as.length, 1); @@ -58,7 +58,7 @@ process.on('exit', onexit); function onexit() { hooks.disable(); - hooks.sanityCheck('HTTPPARSER'); + hooks.sanityCheck('HTTPCLIENTREQUEST'); checkInvocations(httpparser, { init: 1, before: 2, after: 2, destroy: 1 }, 'when process exits'); } diff --git a/test/parallel/test-http-parser.js b/test/parallel/test-http-parser.js index 6d664cfc0adb90..078895d49b13aa 100644 --- a/test/parallel/test-http-parser.js +++ b/test/parallel/test-http-parser.js @@ -95,7 +95,7 @@ function expectBody(expected) { throw new Error('hello world'); }; - parser.reinitialize(HTTPParser.REQUEST, false); + parser.initialize(HTTPParser.REQUEST, request); assert.throws( () => { parser.execute(request, 0, request.length); }, @@ -555,7 +555,7 @@ function expectBody(expected) { parser[kOnBody] = expectBody('ping'); parser.execute(req1, 0, req1.length); - parser.reinitialize(REQUEST, false); + parser.initialize(REQUEST, req2); parser[kOnBody] = expectBody('pong'); parser[kOnHeadersComplete] = onHeadersComplete2; parser.execute(req2, 0, req2.length); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 0818dec2db9c3f..e3cfd6bef01bc9 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -48,6 +48,8 @@ const { getSystemErrorName } = require('util'); if (!common.isMainThread) delete providers.INSPECTORJSBINDING; delete providers.KEYPAIRGENREQUEST; + delete providers.HTTPCLIENTREQUEST; + delete providers.HTTPINCOMINGMESSAGE; const objKeys = Object.keys(providers); if (objKeys.length > 0) diff --git a/test/sequential/test-http-regr-gh-2928.js b/test/sequential/test-http-regr-gh-2928.js index 91db08df47cb97..400dc02013325d 100644 --- a/test/sequential/test-http-regr-gh-2928.js +++ b/test/sequential/test-http-regr-gh-2928.js @@ -7,6 +7,7 @@ const common = require('../common'); const assert = require('assert'); const httpCommon = require('_http_common'); const { HTTPParser } = require('_http_common'); +const { AsyncResource } = require('async_hooks'); const net = require('net'); const COUNT = httpCommon.parsers.max + 1; @@ -24,7 +25,7 @@ function execAndClose() { process.stdout.write('.'); const parser = parsers.pop(); - parser.reinitialize(HTTPParser.RESPONSE, !!parser.reused); + parser.initialize(HTTPParser.RESPONSE, new AsyncResource('ClientRequest')); const socket = net.connect(common.PORT); socket.on('error', (e) => {