Skip to content

Commit a7e49c8

Browse files
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. Fix: #4416 PR-URL: #5419 Reviewed-By: Fedor Indutny <[email protected]>
1 parent da3f425 commit a7e49c8

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
@@ -2,8 +2,8 @@
22
#include "node_buffer.h"
33
#include "node_http_parser.h"
44

5-
#include "base-object.h"
6-
#include "base-object-inl.h"
5+
#include "async-wrap.h"
6+
#include "async-wrap-inl.h"
77
#include "env.h"
88
#include "env-inl.h"
99
#include "stream_base.h"
@@ -147,10 +147,10 @@ struct StringPtr {
147147
};
148148

149149

150-
class Parser : public BaseObject {
150+
class Parser : public AsyncWrap {
151151
public:
152152
Parser(Environment* env, Local<Object> wrap, enum http_parser_type type)
153-
: BaseObject(env, wrap),
153+
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTPPARSER),
154154
current_buffer_len_(0),
155155
current_buffer_data_(nullptr) {
156156
Wrap(object(), this);
@@ -164,6 +164,11 @@ class Parser : public BaseObject {
164164
}
165165

166166

167+
size_t self_size() const override {
168+
return sizeof(*this);
169+
}
170+
171+
167172
HTTP_CB(on_message_begin) {
168173
num_fields_ = num_values_ = 0;
169174
url_.Reset();
@@ -285,8 +290,10 @@ class Parser : public BaseObject {
285290

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

293+
Environment::AsyncCallbackScope callback_scope(env());
294+
288295
Local<Value> head_response =
289-
cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);
296+
MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
290297

291298
if (head_response.IsEmpty()) {
292299
got_exception_ = true;
@@ -321,7 +328,7 @@ class Parser : public BaseObject {
321328
Integer::NewFromUnsigned(env()->isolate(), length)
322329
};
323330

324-
Local<Value> r = cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);
331+
Local<Value> r = MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
325332

326333
if (r.IsEmpty()) {
327334
got_exception_ = true;
@@ -344,7 +351,9 @@ class Parser : public BaseObject {
344351
if (!cb->IsFunction())
345352
return 0;
346353

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

349358
if (r.IsEmpty()) {
350359
got_exception_ = true;
@@ -584,7 +593,7 @@ class Parser : public BaseObject {
584593
parser->current_buffer_len_ = nread;
585594
parser->current_buffer_data_ = buf->base;
586595

587-
cb.As<Function>()->Call(obj, 1, &ret);
596+
parser->MakeCallback(cb.As<Function>(), 1, &ret);
588597

589598
parser->current_buffer_len_ = 0;
590599
parser->current_buffer_data_ = nullptr;
@@ -671,7 +680,7 @@ class Parser : public BaseObject {
671680
url_.ToString(env())
672681
};
673682

674-
Local<Value> r = cb.As<Function>()->Call(obj, ARRAY_SIZE(argv), argv);
683+
Local<Value> r = MakeCallback(cb.As<Function>(), ARRAY_SIZE(argv), argv);
675684

676685
if (r.IsEmpty())
677686
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)