From 97775f1f037193232378272adb48e5d3fa0a9de7 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 13 Jan 2018 17:48:07 +0100
Subject: [PATCH] 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.

PR-URL: https://github.com/nodejs/node/pull/18135
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 lib/_http_common.js     |  5 ++++-
 src/node_http_parser.cc | 24 +-----------------------
 2 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/lib/_http_common.js b/lib/_http_common.js
index cf37bbebe36197..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.
@@ -212,7 +213,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(closeParserInstance, 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 0f17050545528d..9debb8a205ef1c 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -376,8 +376,7 @@ class Parser : public AsyncWrap {
     Parser* parser;
     ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
 
-    if (--parser->refcount_ == 0)
-      delete parser;
+    delete parser;
   }
 
 
@@ -543,22 +542,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) {
@@ -595,8 +578,6 @@ class Parser : public AsyncWrap {
     if (nread == 0)
       return;
 
-    ScopedRetainParser retain(parser);
-
     parser->current_buffer_.Clear();
     Local<Value> ret = parser->Execute(buf->base, nread);
 
@@ -734,7 +715,6 @@ 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;
 
   // These are helper functions for filling `http_parser_settings`, which turn
   // a member function of Parser into a C-style HTTP parser callback.
@@ -751,8 +731,6 @@ class Parser : public AsyncWrap {
   typedef int (Parser::*DataCall)(const char* at, size_t length);
 
   static const struct http_parser_settings settings;
-
-  friend class ScopedRetainParser;
 };
 
 const struct http_parser_settings Parser::settings = {