async_hooks: fixup do not reuse HTTPParser

Fix some issues introduced/not fixed via
https://github.com/nodejs/node/pull/25094:
* Init hook is not emitted for a reused HTTPParser
* HTTPParser was still used as resource in init hook
* type used in init hook was always HTTPINCOMINGMESSAGE even for client
requests
* some tests have not been adapted to new resource names

With this change the async hooks init event is emitted during a call
to Initialize() as the type and resource object is available at this
time. As a result Initialize() must be called now which could be seen
as breaking change even HTTPParser is not part of documented API.

It was needed to put the ClientRequest instance into a wrapper object
instead passing it directly as async resource otherwise
test-domain-multi fails. I think this is because adding an EventEmitter
to a Domain adds a property 'domain' and the presence of this changes
the context propagation in domains.

Besides that tests still refering to resource HTTPParser have been
updated/improved.

Fixes: https://github.com/nodejs/node/issues/27467
Fixes: https://github.com/nodejs/node/issues/26961
Refs: https://github.com/nodejs/node/pull/25094

PR-URL: https://github.com/nodejs/node/pull/27477
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

7 files changed, 68 insertions, 20 deletions

diff --git a/test/async-hooks/coverage.md b/test/async-hooks/coverage.md
index 0a2af0d06b..1ba18a9383 100644
--- a/test/async-hooks/coverage.md
+++ b/test/async-hooks/coverage.md
@@ -9,7 +9,8 @@ Showing which kind of async resource is covered by which test:
 | FSREQCALLBACK        | test-fsreqcallback-{access,readFile}.js    |
 | GETADDRINFOREQWRAP   | test-getaddrinforeqwrap.js                 |
 | GETNAMEINFOREQWRAP   | test-getnameinforeqwrap.js                 |
-| HTTPPARSER           | test-httpparser.{request,response}.js      |
+| HTTPINCOMINGMESSAGE  | test-httpparser.request.js                 |
+| HTTPCLIENTREQUEST    | test-httpparser.response.js                |
 | Immediate            | test-immediate.js                          |
 | JSSTREAM             | TODO (crashes when accessing directly)     |
 | PBKDF2REQUEST        | test-crypto-pbkdf2.js                      |
diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js
index 3461974ef9..55b9b055a0 100644
--- a/test/async-hooks/test-graph.http.js
+++ b/test/async-hooks/test-graph.http.js
@@ -35,13 +35,13 @@ process.on('exit', function() {
       { type: 'TCPCONNECTWRAP',
         id: 'tcpconnect:1',
         triggerAsyncId: 'tcp:1' },
-      { type: 'HTTPPARSER',
-        id: 'httpparser:1',
+      { type: 'HTTPCLIENTREQUEST',
+        id: 'httpclientrequest:1',
         triggerAsyncId: 'tcpserver:1' },
       { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' },
       { type: 'Timeout', id: 'timeout:1', triggerAsyncId: 'tcp:2' },
-      { type: 'HTTPPARSER',
-        id: 'httpparser:2',
+      { type: 'HTTPINCOMINGMESSAGE',
+        id: 'httpincomingmessage:1',
         triggerAsyncId: 'tcp:2' },
       { type: 'Timeout',
         id: 'timeout:2',
diff --git a/test/async-hooks/test-graph.tls-write.js b/test/async-hooks/test-graph.tls-write.js
index 580264316d..5aee38e6b6 100644
--- a/test/async-hooks/test-graph.tls-write.js
+++ b/test/async-hooks/test-graph.tls-write.js
@@ -64,12 +64,8 @@ function onexit() {
         id: 'getaddrinforeq:1', triggerAsyncId: 'tls:1' },
       { type: 'TCPCONNECTWRAP',
         id: 'tcpconnect:1', triggerAsyncId: 'tcp:1' },
-      { type: 'WRITEWRAP', id: 'write:1', triggerAsyncId: 'tcpconnect:1' },
       { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' },
       { type: 'TLSWRAP', id: 'tls:2', triggerAsyncId: 'tcpserver:1' },
-      { type: 'WRITEWRAP', id: 'write:2', triggerAsyncId: null },
-      { type: 'WRITEWRAP', id: 'write:3', triggerAsyncId: null },
-      { type: 'WRITEWRAP', id: 'write:4', triggerAsyncId: null },
       { type: 'Immediate', id: 'immediate:1', triggerAsyncId: 'tcp:2' },
       { type: 'Immediate', id: 'immediate:2', triggerAsyncId: 'tcp:1' },
     ]
diff --git a/test/async-hooks/test-httparser-reuse.js b/test/async-hooks/test-httparser-reuse.js
index b6d82d7d5e..06441562e0 100644
--- a/test/async-hooks/test-httparser-reuse.js
+++ b/test/async-hooks/test-httparser-reuse.js
@@ -1,28 +1,54 @@
 'use strict';
 
 const common = require('../common');
-const http = require('http');
 const assert = require('assert');
 const { createHook } = require('async_hooks');
+const http = require('http');
+
+// Verify that resource emitted for an HTTPParser is not reused.
+// Verify that correct create/destroy events are emitted.
+
 const reused = Symbol('reused');
 
-let reusedHTTPParser = false;
-const asyncHook = createHook({
+const reusedParser = [];
+const incomingMessageParser = [];
+const clientRequestParser = [];
+const dupDestroys = [];
+const destroyed = [];
+
+createHook({
   init(asyncId, type, triggerAsyncId, resource) {
+    switch (type) {
+      case 'HTTPINCOMINGMESSAGE':
+        incomingMessageParser.push(asyncId);
+        break;
+      case 'HTTPCLIENTREQUEST':
+        clientRequestParser.push(asyncId);
+        break;
+    }
+
     if (resource[reused]) {
-      reusedHTTPParser = true;
+      reusedParser.push(
+        `resource reused: ${asyncId}, ${triggerAsyncId}, ${type}`
+      );
     }
     resource[reused] = true;
+  },
+  destroy(asyncId) {
+    if (destroyed.includes(asyncId)) {
+      dupDestroys.push(asyncId);
+    } else {
+      destroyed.push(asyncId);
+    }
   }
-});
-asyncHook.enable();
+}).enable();
 
-const server = http.createServer(function(req, res) {
+const server = http.createServer((req, res) => {
   res.end();
 });
 
 const PORT = 3000;
-const url = 'http://127.0.0.1:' + PORT;
+const url = `http://127.0.0.1:${PORT}`;
 
 server.listen(PORT, common.mustCall(() => {
   http.get(url, common.mustCall(() => {
@@ -30,10 +56,21 @@ server.listen(PORT, common.mustCall(() => {
       server.listen(PORT, common.mustCall(() => {
         http.get(url, common.mustCall(() => {
           server.close(common.mustCall(() => {
-            assert.strictEqual(reusedHTTPParser, false);
+            setTimeout(common.mustCall(verify), 200);
           }));
         }));
       }));
     }));
   }));
 }));
+
+function verify() {
+  assert.strictEqual(reusedParser.length, 0);
+
+  assert.strictEqual(incomingMessageParser.length, 2);
+  assert.strictEqual(clientRequestParser.length, 2);
+
+  assert.strictEqual(dupDestroys.length, 0);
+  incomingMessageParser.forEach((id) => assert.ok(destroyed.includes(id)));
+  clientRequestParser.forEach((id) => assert.ok(destroyed.includes(id)));
+}
diff --git a/test/async-hooks/test-httpparser.request.js b/test/async-hooks/test-httpparser.request.js
index 7ba8feefe3..f4552398d3 100644
--- a/test/async-hooks/test-httpparser.request.js
+++ b/test/async-hooks/test-httpparser.request.js
@@ -20,7 +20,8 @@ const request = Buffer.from(
   'GET /hello HTTP/1.1\r\n\r\n'
 );
 
-const parser = new HTTPParser(REQUEST);
+const parser = new HTTPParser();
+parser.initialize(REQUEST, {});
 const as = hooks.activitiesOfTypes('HTTPINCOMINGMESSAGE');
 const httpparser = as[0];
 
diff --git a/test/async-hooks/test-httpparser.response.js b/test/async-hooks/test-httpparser.response.js
index 85d6f76525..a207a62636 100644
--- a/test/async-hooks/test-httpparser.response.js
+++ b/test/async-hooks/test-httpparser.response.js
@@ -25,7 +25,8 @@ const request = Buffer.from(
   'pong'
 );
 
-const parser = new HTTPParser(RESPONSE);
+const parser = new HTTPParser();
+parser.initialize(RESPONSE, {});
 const as = hooks.activitiesOfTypes('HTTPCLIENTREQUEST');
 const httpparser = as[0];
 
diff --git a/test/async-hooks/verify-graph.js b/test/async-hooks/verify-graph.js
index d1e09f92bc..1b188faa14 100644
--- a/test/async-hooks/verify-graph.js
+++ b/test/async-hooks/verify-graph.js
@@ -98,6 +98,18 @@ module.exports = function verifyGraph(hooks, graph) {
     );
   }
   assert.strictEqual(errors.length, 0);
+
+  // Verify that all expected types are present
+  const expTypes = Object.create(null);
+  for (let i = 0; i < graph.length; i++) {
+    if (expTypes[graph[i].type] == null) expTypes[graph[i].type] = 0;
+    expTypes[graph[i].type]++;
+  }
+
+  for (const type in expTypes) {
+    assert.strictEqual(typeSeen[type], expTypes[type],
+                       `Expecting type '${type}' in graph`);
+  }
 };
 
 //