From a368b43a106c6540af0ac4bdaaac648599a72fda Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 13 Jan 2018 17:48:07 +0100
Subject: [PATCH 1/2] http: simplify parser lifetime tracking

Instead of providing a separate class for keeping the
parser alive during its own call back, just delay a
possible `.close()` call until the stack has cleared
completely.
---
 lib/_http_common.js     |  4 +++-
 src/node_http_parser.cc | 24 +-----------------------
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/lib/_http_common.js b/lib/_http_common.js
index cf37bbebe36197..94840efd0e4874 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -212,7 +212,9 @@ function freeParser(parser, req, socket) {
     parser.outgoing = null;
     parser[kOnExecute] = null;
     if (parsers.free(parser) === false) {
-      parser.close();
+      // Make sure the parser's stack has unwound before deleting the
+      // corresponding C++ object through .close().
+      setImmediate((parser) => parser.close(), parser);
     } else {
       // Since the Parser destructor isn't going to run the destroy() callbacks
       // it needs to be triggered manually.
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index f378a0475a65c0..46519180619f87 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -392,8 +392,7 @@ class Parser : public AsyncWrap {
     Parser* parser;
     ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
 
-    if (--parser->refcount_ == 0)
-      delete parser;
+    delete parser;
   }
 
 
@@ -559,22 +558,6 @@ class Parser : public AsyncWrap {
   }
 
  protected:
-  class ScopedRetainParser {
-   public:
-    explicit ScopedRetainParser(Parser* p) : p_(p) {
-      CHECK_GT(p_->refcount_, 0);
-      p_->refcount_++;
-    }
-
-    ~ScopedRetainParser() {
-      if (0 == --p_->refcount_)
-        delete p_;
-    }
-
-   private:
-    Parser* const p_;
-  };
-
   static const size_t kAllocBufferSize = 64 * 1024;
 
   static void OnAllocImpl(size_t suggested_size, uv_buf_t* buf, void* ctx) {
@@ -611,8 +594,6 @@ class Parser : public AsyncWrap {
     if (nread == 0)
       return;
 
-    ScopedRetainParser retain(parser);
-
     parser->current_buffer_.Clear();
     Local<Value> ret = parser->Execute(buf->base, nread);
 
@@ -750,10 +731,7 @@ class Parser : public AsyncWrap {
   char* current_buffer_data_;
   StreamResource::Callback<StreamResource::AllocCb> prev_alloc_cb_;
   StreamResource::Callback<StreamResource::ReadCb> prev_read_cb_;
-  int refcount_ = 1;
   static const struct http_parser_settings settings;
-
-  friend class ScopedRetainParser;
 };
 
 

From 5067b2489472dca771c43d9181bc9b952aa7bdcc Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 13 Jan 2018 18:25:35 +0100
Subject: [PATCH 2/2] [squash] jasnell nit

---
 lib/_http_common.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/_http_common.js b/lib/_http_common.js
index 94840efd0e4874..6ad13104772ede 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -190,6 +190,7 @@ var parsers = new FreeList('parsers', 1000, function() {
   return parser;
 });
 
+function closeParserInstance(parser) { parser.close(); }
 
 // Free the parser and also break any links that it
 // might have to any other things.
@@ -214,7 +215,7 @@ function freeParser(parser, req, socket) {
     if (parsers.free(parser) === false) {
       // Make sure the parser's stack has unwound before deleting the
       // corresponding C++ object through .close().
-      setImmediate((parser) => parser.close(), parser);
+      setImmediate(closeParserInstance, parser);
     } else {
       // Since the Parser destructor isn't going to run the destroy() callbacks
       // it needs to be triggered manually.