From a7e49c886f5cb8c351673f413dc66086ff1d75bc Mon Sep 17 00:00:00 2001
From: Trevor Norris <trev.norris@gmail.com>
Date: Fri, 19 Feb 2016 13:13:04 -0700
Subject: [PATCH] http_parser: use `MakeCallback`

Make `HTTPParser` an instance of `AsyncWrap` and make it use
`MakeCallback`. This means that async wrap hooks will be called on
consumed TCP sockets as well as on non-consumed ones.

Additional uses of `AsyncCallbackScope` are necessary to prevent
improper state from progressing that triggers failure in the
test-http-pipeline-flood.js test. Optimally this wouldn't be necessary,
but for the time being it's the most sure way to allow operations to
proceed as they have.

Fix: https://github.com/nodejs/node/issues/4416
PR-URL: https://github.com/nodejs/node/pull/5419
Reviewed-By: Fedor Indutny <fedor@indutny.com>
---
 src/async-wrap.h                              |  1 +
 src/node_http_parser.cc                       | 27 ++++++++++++-------
 .../test-async-wrap-check-providers.js        |  3 +++
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/src/async-wrap.h b/src/async-wrap.h
index 5db29600bcd180..cb0c9e211a8923 100644
--- a/src/async-wrap.h
+++ b/src/async-wrap.h
@@ -17,6 +17,7 @@ namespace node {
   V(FSREQWRAP)                                                                \
   V(GETADDRINFOREQWRAP)                                                       \
   V(GETNAMEINFOREQWRAP)                                                       \
+  V(HTTPPARSER)                                                               \
   V(JSSTREAM)                                                                 \
   V(PIPEWRAP)                                                                 \
   V(PIPECONNECTWRAP)                                                          \
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 12c6fcff86c145..0b363ca6c310f7 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -2,8 +2,8 @@
 #include "node_buffer.h"
 #include "node_http_parser.h"
 
-#include "base-object.h"
-#include "base-object-inl.h"
+#include "async-wrap.h"
+#include "async-wrap-inl.h"
 #include "env.h"
 #include "env-inl.h"
 #include "stream_base.h"
@@ -147,10 +147,10 @@ struct StringPtr {
 };
 
 
-class Parser : public BaseObject {
+class Parser : public AsyncWrap {
  public:
   Parser(Environment* env, Local<Object> wrap, enum http_parser_type type)
-      : BaseObject(env, wrap),
+      : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTPPARSER),
         current_buffer_len_(0),
         current_buffer_data_(nullptr) {
     Wrap(object(), this);
@@ -164,6 +164,11 @@ class Parser : public BaseObject {
   }
 
 
+  size_t self_size() const override {
+    return sizeof(*this);
+  }
+
+
   HTTP_CB(on_message_begin) {
     num_fields_ = num_values_ = 0;
     url_.Reset();
@@ -285,8 +290,10 @@ class Parser : public BaseObject {
 
     argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);
 
+    Environment::AsyncCallbackScope callback_scope(env());
+
     Local<Value> head_response =
-        cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);
+        MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
 
     if (head_response.IsEmpty()) {
       got_exception_ = true;
@@ -321,7 +328,7 @@ class Parser : public BaseObject {
       Integer::NewFromUnsigned(env()->isolate(), length)
     };
 
-    Local<Value> r = cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);
+    Local<Value> r = MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
 
     if (r.IsEmpty()) {
       got_exception_ = true;
@@ -344,7 +351,9 @@ class Parser : public BaseObject {
     if (!cb->IsFunction())
       return 0;
 
-    Local<Value> r = cb.As<Function>()->Call(obj, 0, nullptr);
+    Environment::AsyncCallbackScope callback_scope(env());
+
+    Local<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
 
     if (r.IsEmpty()) {
       got_exception_ = true;
@@ -584,7 +593,7 @@ class Parser : public BaseObject {
     parser->current_buffer_len_ = nread;
     parser->current_buffer_data_ = buf->base;
 
-    cb.As<Function>()->Call(obj, 1, &ret);
+    parser->MakeCallback(cb.As<Function>(), 1, &ret);
 
     parser->current_buffer_len_ = 0;
     parser->current_buffer_data_ = nullptr;
@@ -671,7 +680,7 @@ class Parser : public BaseObject {
       url_.ToString(env())
     };
 
-    Local<Value> r = cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);
+    Local<Value> r = MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
 
     if (r.IsEmpty())
       got_exception_ = true;
diff --git a/test/parallel/test-async-wrap-check-providers.js b/test/parallel/test-async-wrap-check-providers.js
index c08c7a43d445e0..3a2c80d9cbe173 100644
--- a/test/parallel/test-async-wrap-check-providers.js
+++ b/test/parallel/test-async-wrap-check-providers.js
@@ -11,6 +11,7 @@ const tls = require('tls');
 const zlib = require('zlib');
 const ChildProcess = require('child_process').ChildProcess;
 const StreamWrap = require('_stream_wrap').StreamWrap;
+const HTTPParser = process.binding('http_parser').HTTPParser;
 const async_wrap = process.binding('async_wrap');
 const pkeys = Object.keys(async_wrap.Providers);
 
@@ -106,6 +107,8 @@ zlib.createGzip();
 
 new ChildProcess();
 
+new HTTPParser(HTTPParser.REQUEST);
+
 process.on('exit', function() {
   if (keyList.length !== 0) {
     process._rawDebug('Not all keys have been used:');