1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

http_parser: use MakeCallback

Browse Source 
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>
This commit is contained in:
Trevor Norris 2016-02-19 13:13:04 -07:00
parent da3f425506
commit a7e49c886f
3 changed files with 22 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/async-wrap.h
View File

@ -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
View File

@ -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;

3
test/parallel/test-async-wrap-check-providers.js
View File

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