Browse Source

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.

Ref: https://github.com/nodejs/node/pull/7048
Fix: https://github.com/nodejs/node/issues/4416
PR-URL: https://github.com/nodejs/node/pull/5419
Reviewed-By: Fedor Indutny <fedor@indutny.com>
v4.x
Trevor Norris 9 years ago
committed by Myles Borins
parent
commit
6f312b3a91
  1. 1
      src/async-wrap.h
  2. 27
      src/node_http_parser.cc
  3. 3
      test/parallel/test-async-wrap-check-providers.js

1
src/async-wrap.h

@ -17,6 +17,7 @@ namespace node {
V(FSREQWRAP) \
V(GETADDRINFOREQWRAP) \
V(GETNAMEINFOREQWRAP) \
V(HTTPPARSER) \
V(JSSTREAM) \
V(PIPEWRAP) \
V(PIPECONNECTWRAP) \

27
src/node_http_parser.cc

@ -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"
@ -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);
@ -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();
@ -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;
@ -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;
@ -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;
@ -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;
@ -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;

3
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:');

Loading…
Cancel
Save