async_hooks: add missing async_hooks destroys in AsyncReset

This adds missing async_hooks destroy calls for sockets (in
_http_agent.js) and HTTP parsers. We need to emit a destroy in
AsyncWrap#AsyncReset before assigning a new async_id when the instance
has already been in use and is being recycled, because in that case, we
have already emitted an init for the "old" async_id.

This also removes a duplicated init call for HTTP parser: Each time a
new parser was created, AsyncReset was being called via the C++ Parser
class constructor (super constructor AsyncWrap) and also via
Parser::Reinitialize.

PR-URL: https://github.com/nodejs/node/pull/23272
Fixes: https://github.com/nodejs/node/issues/19859
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

5 files changed, 23 insertions, 8 deletions

diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index 1a2920cf09..97c5ab604f 100644
--- a/lib/_http_agent.js
+++ b/lib/_http_agent.js
@@ -167,7 +167,7 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */,
     var socket = this.freeSockets[name].shift();
     // Guard against an uninitialized or user supplied Socket.
     if (socket._handle && typeof socket._handle.asyncReset === 'function') {
-      // Assign the handle a new asyncId and run any init() hooks.
+      // Assign the handle a new asyncId and run any destroy()/init() hooks.
       socket._handle.asyncReset();
       socket[async_id_symbol] = socket._handle.getAsyncId();
     }
diff --git a/lib/_http_client.js b/lib/_http_client.js
index c83800a93b..d91b43516f 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -47,6 +47,7 @@ const {
   ERR_UNESCAPED_CHARACTERS
 } = require('internal/errors').codes;
 const { validateTimerDuration } = require('internal/timers');
+const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol;
 
 const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
 
@@ -631,7 +632,7 @@ function tickOnSocket(req, socket) {
   var parser = parsers.alloc();
   req.socket = socket;
   req.connection = socket;
-  parser.reinitialize(HTTPParser.RESPONSE);
+  parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
   parser.socket = socket;
   parser.outgoing = req;
   req.parser = parser;
diff --git a/lib/_http_common.js b/lib/_http_common.js
index 1de0ee6025..b37814f783 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -23,7 +23,7 @@
 
 const { methods, HTTPParser } = internalBinding('http_parser');
 
-const FreeList = require('internal/freelist');
+const { FreeList } = require('internal/freelist');
 const { ondrain } = require('internal/http');
 const incoming = require('_http_incoming');
 const {
diff --git a/lib/_http_server.js b/lib/_http_server.js
index cc1a428cd6..3b2d7f5041 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -42,6 +42,7 @@ const {
   defaultTriggerAsyncIdScope,
   getOrSetAsyncId
 } = require('internal/async_hooks');
+const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol;
 const { IncomingMessage } = require('_http_incoming');
 const {
   ERR_HTTP_HEADERS_SENT,
@@ -338,7 +339,7 @@ function connectionListenerInternal(server, socket) {
   socket.on('timeout', socketOnTimeout);
 
   var parser = parsers.alloc();
-  parser.reinitialize(HTTPParser.REQUEST);
+  parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]);
   parser.socket = socket;
   socket.parser = parser;
 
diff --git a/lib/internal/freelist.js b/lib/internal/freelist.js
index 7e9cef9528..04d684e833 100644
--- a/lib/internal/freelist.js
+++ b/lib/internal/freelist.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const is_reused_symbol = Symbol('isReused');
+
 class FreeList {
   constructor(name, max, ctor) {
     this.name = name;
@@ -9,9 +11,15 @@ class FreeList {
   }
 
   alloc() {
-    return this.list.length ?
-      this.list.pop() :
-      this.ctor.apply(this, arguments);
+    let item;
+    if (this.list.length > 0) {
+      item = this.list.pop();
+      item[is_reused_symbol] = true;
+    } else {
+      item = this.ctor.apply(this, arguments);
+      item[is_reused_symbol] = false;
+    }
+    return item;
   }
 
   free(obj) {
@@ -23,4 +31,9 @@ class FreeList {
   }
 }
 
-module.exports = FreeList;
+module.exports = {
+  FreeList,
+  symbols: {
+    is_reused_symbol
+  }
+};