Skip to content

Commit

Permalink
http_parser: use MakeCallback
Browse files Browse the repository at this point in the history
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.

Ref: #7048
Fix: #4416
PR-URL: #5419
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
trevnorris authored and Myles Borins committed Jun 24, 2016
1 parent aa64256 commit e7262cb
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace node {
V(FSREQWRAP) \
V(GETADDRINFOREQWRAP) \
V(GETNAMEINFOREQWRAP) \
V(HTTPPARSER) \
V(JSSTREAM) \
V(PIPEWRAP) \
V(PIPECONNECTWRAP) \
Expand Down
27 changes: 18 additions & 9 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
#include "node_http_parser.h"
#include "node_revert.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"
Expand Down Expand Up @@ -148,10 +148,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);
Expand All @@ -165,6 +165,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();
Expand Down Expand Up @@ -286,8 +291,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, arraysize(argv), argv);
MakeCallback(cb.As<Function>(), arraysize(argv), argv);

if (head_response.IsEmpty()) {
got_exception_ = true;
Expand Down Expand Up @@ -322,7 +329,7 @@ class Parser : public BaseObject {
Integer::NewFromUnsigned(env()->isolate(), length)
};

Local<Value> r = cb.As<Function>()->Call(obj, arraysize(argv), argv);
Local<Value> r = MakeCallback(cb.As<Function>(), arraysize(argv), argv);

if (r.IsEmpty()) {
got_exception_ = true;
Expand All @@ -345,7 +352,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;
Expand Down Expand Up @@ -585,7 +594,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;
Expand Down Expand Up @@ -659,7 +668,7 @@ class Parser : public BaseObject {
url_.ToString(env())
};

Local<Value> r = cb.As<Function>()->Call(obj, arraysize(argv), argv);
Local<Value> r = MakeCallback(cb.As<Function>(), arraysize(argv), argv);

if (r.IsEmpty())
got_exception_ = true;
Expand Down
3 changes: 3 additions & 0 deletions test/parallel/test-async-wrap-check-providers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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:');
Expand Down

0 comments on commit e7262cb

Please sign in to comment.