From 3d41d4f74f1d63fa8d9224b823c72194c18101cc Mon Sep 17 00:00:00 2001
From: Bastian Krol <bastian.krol@instana.com>
Date: Fri, 5 Oct 2018 08:46:35 +0200
Subject: [PATCH] http: emit destroy for reused AsyncWraps from C++

Refs: #19859
---
 lib/_http_client.js     |  9 +--------
 lib/_http_server.js     |  5 -----
 src/async_wrap.cc       | 16 ++++++++++++++--
 src/async_wrap.h        |  4 +++-
 src/node_http_parser.cc |  2 +-
 5 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/lib/_http_client.js b/lib/_http_client.js
index ccfaea03d56c5e..d91b43516fa4ee 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -36,11 +36,7 @@ const {
 const { OutgoingMessage } = require('_http_outgoing');
 const Agent = require('_http_agent');
 const { Buffer } = require('buffer');
-const {
-  defaultTriggerAsyncIdScope,
-  destroyHooksExist,
-  emitDestroy
-} = require('internal/async_hooks');
+const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
 const { URL, urlToOptions, searchParamsSymbol } = require('internal/url');
 const { outHeadersKey, ondrain } = require('internal/http');
 const {
@@ -636,9 +632,6 @@ function tickOnSocket(req, socket) {
   var parser = parsers.alloc();
   req.socket = socket;
   req.connection = socket;
-  if (destroyHooksExist() && parser[is_reused_symbol] && parser.getAsyncId()) {
-    emitDestroy(parser.getAsyncId());
-  }
   parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
   parser.socket = socket;
   parser.outgoing = req;
diff --git a/lib/_http_server.js b/lib/_http_server.js
index b45ce78c4ed63c..3b2d7f50419127 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -40,8 +40,6 @@ const { OutgoingMessage } = require('_http_outgoing');
 const { outHeadersKey, ondrain } = require('internal/http');
 const {
   defaultTriggerAsyncIdScope,
-  destroyHooksExist,
-  emitDestroy,
   getOrSetAsyncId
 } = require('internal/async_hooks');
 const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol;
@@ -341,9 +339,6 @@ function connectionListenerInternal(server, socket) {
   socket.on('timeout', socketOnTimeout);
 
   var parser = parsers.alloc();
-  if (destroyHooksExist() && parser[is_reused_symbol] && parser.getAsyncId()) {
-    emitDestroy(parser.getAsyncId());
-  }
   parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]);
   parser.socket = socket;
   socket.parser = parser;
diff --git a/src/async_wrap.cc b/src/async_wrap.cc
index 2b163a5fa283d1..a8d2faae523a27 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -413,7 +413,10 @@ void AsyncWrap::AsyncReset(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
   double execution_async_id =
       args[0]->IsNumber() ? args[0].As<Number>()->Value() : -1;
-  wrap->AsyncReset(execution_async_id);
+  // This assumes that when asyncReset is called from JS, it is always about
+  // reusing an existing AsyncWrap instance and never to initialize a freshly
+  // created AsyncWrap instance.
+  wrap->AsyncReset(execution_async_id, false, true);
 }
 
 
@@ -605,7 +608,16 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
 // 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(double execution_async_id,
+                           bool silent,
+                           bool reused) {
+  if (reused) {
+    // If 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
+    // before generating a new async_id.
+    EmitDestroy(env(), get_async_id());
+  }
+
   async_id_ =
     execution_async_id == -1 ? env()->new_async_id() : execution_async_id;
   trigger_async_id_ = env()->get_default_trigger_async_id();
diff --git a/src/async_wrap.h b/src/async_wrap.h
index 360380afc3459b..3e9307b8c13798 100644
--- a/src/async_wrap.h
+++ b/src/async_wrap.h
@@ -147,7 +147,9 @@ class AsyncWrap : public BaseObject {
 
   inline double get_trigger_async_id() const;
 
-  void AsyncReset(double execution_async_id = -1, bool silent = false);
+  void AsyncReset(double execution_async_id = -1,
+                  bool silent = false,
+                  bool reused = false);
 
   // Only call these within a valid HandleScope.
   v8::MaybeLocal<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 9850b4f698205b..5b232b92199d21 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -479,7 +479,7 @@ class Parser : public AsyncWrap, public StreamListener {
     // 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();
+      parser->AsyncReset(-1, false, true);
     }
     parser->Init(type);
   }