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: #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>

Author: addaleax

lib/_http_common.js

@@ -186,6 +186,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.
@@ -208,7 +209,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.

src/node_http_parser.cc

@@ -378,8 +378,7 @@ class Parser : public AsyncWrap {
     Parser* parser;
     ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
 
-    if (--parser->refcount_ == 0)
-      delete parser;
+    delete parser;
   }
 
 
@@ -545,22 +544,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) {
@@ -597,8 +580,6 @@ class Parser : public AsyncWrap {
     if (nread == 0)
       return;
 
-    ScopedRetainParser retain(parser);
-
     parser->current_buffer_.Clear();
     Local<Value> ret = parser->Execute(buf->base, nread);
 
@@ -736,7 +717,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.
@@ -753,8 +733,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 = {