Skip to content

Commit 88caad4

Browse files
trevnorrisMyles Borins
authored and
Myles Borins
committed
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: #7048 Fix: #4416 PR-URL: #5419 Reviewed-By: Fedor Indutny <[email protected]>
1 parent 4c975b0 commit 88caad4

File tree

3 files changed

+22
-9
lines changed

3 files changed

+22
-9
lines changed

src/async-wrap.h

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace node {
1717
V(FSREQWRAP) \
1818
V(GETADDRINFOREQWRAP) \
1919
V(GETNAMEINFOREQWRAP) \
20+
V(HTTPPARSER) \
2021
V(JSSTREAM) \
2122
V(PIPEWRAP) \
2223
V(PIPECONNECTWRAP) \

src/node_http_parser.cc

+18-9
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
#include "node_http_parser.h"
44
#include "node_revert.h"
55

6-
#include "base-object.h"
7-
#include "base-object-inl.h"
6+
#include "async-wrap.h"
7+
#include "async-wrap-inl.h"
88
#include "env.h"
99
#include "env-inl.h"
1010
#include "stream_base.h"
@@ -148,10 +148,10 @@ struct StringPtr {
148148
};
149149

150150

151-
class Parser : public BaseObject {
151+
class Parser : public AsyncWrap {
152152
public:
153153
Parser(Environment* env, Local<Object> wrap, enum http_parser_type type)
154-
: BaseObject(env, wrap),
154+
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTPPARSER),
155155
current_buffer_len_(0),
156156
current_buffer_data_(nullptr) {
157157
Wrap(object(), this);
@@ -165,6 +165,11 @@ class Parser : public BaseObject {
165165
}
166166

167167

168+
size_t self_size() const override {
169+
return sizeof(*this);
170+
}
171+
172+
168173
HTTP_CB(on_message_begin) {
169174
num_fields_ = num_values_ = 0;
170175
url_.Reset();
@@ -286,8 +291,10 @@ class Parser : public BaseObject {
286291

287292
argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);
288293

294+
Environment::AsyncCallbackScope callback_scope(env());
295+
289296
Local<Value> head_response =
290-
cb.As<Function>()->Call(obj, arraysize(argv), argv);
297+
MakeCallback(cb.As<Function>(), arraysize(argv), argv);
291298

292299
if (head_response.IsEmpty()) {
293300
got_exception_ = true;
@@ -322,7 +329,7 @@ class Parser : public BaseObject {
322329
Integer::NewFromUnsigned(env()->isolate(), length)
323330
};
324331

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

327334
if (r.IsEmpty()) {
328335
got_exception_ = true;
@@ -345,7 +352,9 @@ class Parser : public BaseObject {
345352
if (!cb->IsFunction())
346353
return 0;
347354

348-
Local<Value> r = cb.As<Function>()->Call(obj, 0, nullptr);
355+
Environment::AsyncCallbackScope callback_scope(env());
356+
357+
Local<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
349358

350359
if (r.IsEmpty()) {
351360
got_exception_ = true;
@@ -585,7 +594,7 @@ class Parser : public BaseObject {
585594
parser->current_buffer_len_ = nread;
586595
parser->current_buffer_data_ = buf->base;
587596

588-
cb.As<Function>()->Call(obj, 1, &ret);
597+
parser->MakeCallback(cb.As<Function>(), 1, &ret);
589598

590599
parser->current_buffer_len_ = 0;
591600
parser->current_buffer_data_ = nullptr;
@@ -659,7 +668,7 @@ class Parser : public BaseObject {
659668
url_.ToString(env())
660669
};
661670

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

664673
if (r.IsEmpty())
665674
got_exception_ = true;

test/parallel/test-async-wrap-check-providers.js

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const tls = require('tls');
1111
const zlib = require('zlib');
1212
const ChildProcess = require('child_process').ChildProcess;
1313
const StreamWrap = require('_stream_wrap').StreamWrap;
14+
const HTTPParser = process.binding('http_parser').HTTPParser;
1415
const async_wrap = process.binding('async_wrap');
1516
const pkeys = Object.keys(async_wrap.Providers);
1617

@@ -106,6 +107,8 @@ zlib.createGzip();
106107

107108
new ChildProcess();
108109

110+
new HTTPParser(HTTPParser.REQUEST);
111+
109112
process.on('exit', function() {
110113
if (keyList.length !== 0) {
111114
process._rawDebug('Not all keys have been used:');

0 commit comments

Comments
 (0)