Skip to content

Commit d591884

Browse files
anonrigJonas Badalic
anonrig
and
Jonas Badalic
committed
http: remove async tracking
Co-Authored-by: Jonas Badalic <jonas.badalic@gmail.com>
1 parent 255dd7b commit d591884

20 files changed

+96
-507
lines changed

‎benchmark/http/bench-parser.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ function main({ len, n }) {
2323
bench.start();
2424
for (let i = 0; i < n; i++) {
2525
parser.execute(header, 0, header.length);
26-
parser.initialize(REQUEST, {});
26+
parser.initialize(REQUEST);
2727
}
2828
bench.end(n);
2929
}
3030

3131
function newParser(type) {
3232
const parser = new HTTPParser();
33-
parser.initialize(type, {});
33+
parser.initialize(type);
3434

3535
parser.headers = [];
3636

‎lib/_http_client.js‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,6 @@ function validateHost(host, name) {
130130
return host;
131131
}
132132

133-
class HTTPClientAsyncResource {
134-
constructor(type, req) {
135-
this.type = type;
136-
this.req = req;
137-
}
138-
}
139-
140133
// When proxying a HTTP request, the following needs to be done:
141134
// https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2
142135
// 1. Rewrite the request path to absolute-form.
@@ -880,7 +873,6 @@ function tickOnSocket(req, socket) {
880873
const lenient = req.insecureHTTPParser === undefined ?
881874
isLenient() : req.insecureHTTPParser;
882875
parser.initialize(HTTPParser.RESPONSE,
883-
new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
884876
req.maxHeaderSize || 0,
885877
lenient ? kLenientAll : kLenientNone);
886878
parser.socket = socket;

‎lib/_http_common.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,6 @@ function freeParser(parser, req, socket) {
187187
// Make sure the parser's stack has unwound before deleting the
188188
// corresponding C++ object through .close().
189189
setImmediate(closeParserInstance, parser);
190-
} else {
191-
// Since the Parser destructor isn't going to run the destroy() callbacks
192-
// it needs to be triggered manually.
193-
parser.free();
194190
}
195191
}
196192
if (req) {

‎lib/_http_server.js‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,6 @@ const kConnectionsCheckingInterval = Symbol('http.server.connectionsCheckingInte
187187

188188
const HTTP_SERVER_TRACE_EVENT_NAME = 'http.server.request';
189189

190-
class HTTPServerAsyncResource {
191-
constructor(type, socket) {
192-
this.type = type;
193-
this.socket = socket;
194-
}
195-
}
196-
197190
function ServerResponse(req, options) {
198191
OutgoingMessage.call(this, options);
199192

@@ -705,7 +698,6 @@ function connectionListenerInternal(server, socket) {
705698
// https://github.com/nodejs/node/pull/21313
706699
parser.initialize(
707700
HTTPParser.REQUEST,
708-
new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
709701
server.maxHeaderSize || 0,
710702
lenient ? kLenientAll : kLenientNone,
711703
server[kConnections],

‎src/async_wrap.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ namespace node {
4949
V(HTTP2STREAM) \
5050
V(HTTP2PING) \
5151
V(HTTP2SETTINGS) \
52-
V(HTTPINCOMINGMESSAGE) \
53-
V(HTTPCLIENTREQUEST) \
5452
V(LOCKS) \
5553
V(JSSTREAM) \
5654
V(JSUDPWRAP) \

‎src/node_http_parser.cc‎

Lines changed: 51 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

22+
#include "js_native_api_v8.h"
2223
#include "node.h"
2324
#include "node_buffer.h"
2425
#include "util.h"
2526

26-
#include "async_wrap-inl.h"
2727
#include "env-inl.h"
2828
#include "llhttp.h"
2929
#include "memory_tracker-inl.h"
@@ -64,7 +64,6 @@ using v8::Integer;
6464
using v8::Isolate;
6565
using v8::Local;
6666
using v8::LocalVector;
67-
using v8::MaybeLocal;
6867
using v8::Number;
6968
using v8::Object;
7069
using v8::ObjectTemplate;
@@ -250,17 +249,16 @@ class ConnectionsList : public BaseObject {
250249
std::set<Parser*, ParserComparator> active_connections_;
251250
};
252251

253-
class Parser : public AsyncWrap, public StreamListener {
252+
class Parser : public BaseObject, public StreamListener {
254253
friend class ConnectionsList;
255254
friend struct ParserComparator;
256255

257256
public:
258257
Parser(BindingData* binding_data, Local<Object> wrap)
259-
: AsyncWrap(binding_data->env(), wrap),
258+
: BaseObject(binding_data->env(), wrap),
260259
current_buffer_len_(0),
261260
current_buffer_data_(nullptr),
262-
binding_data_(binding_data) {
263-
}
261+
binding_data_(binding_data) {}
264262

265263
SET_NO_MEMORY_INFO()
266264
SET_MEMORY_INFO_NAME(Parser)
@@ -286,16 +284,20 @@ class Parser : public AsyncWrap, public StreamListener {
286284
connectionsList_->PushActive(this);
287285
}
288286

289-
Local<Value> cb = object()->Get(env()->context(), kOnMessageBegin)
290-
.ToLocalChecked();
291-
if (cb->IsFunction()) {
292-
InternalCallbackScope callback_scope(
293-
this, InternalCallbackScope::kSkipTaskQueues);
294-
295-
MaybeLocal<Value> r = cb.As<Function>()->Call(
296-
env()->context(), object(), 0, nullptr);
287+
auto context = env()->context();
297288

298-
if (r.IsEmpty()) callback_scope.MarkAsFailed();
289+
Local<Value> cb = object()->Get(context, kOnMessageBegin).ToLocalChecked();
290+
if (cb->IsFunction()) {
291+
v8::TryCatch try_catch(env()->isolate());
292+
USE(cb.As<Function>()->Call(context, object(), 0, nullptr));
293+
294+
if (try_catch.HasCaught()) {
295+
got_exception_ = true;
296+
// This is part of http parsing flow. We need to set the proper error
297+
// reason for the parser.
298+
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
299+
return HPE_USER;
300+
}
299301
}
300302

301303
return 0;
@@ -442,15 +444,14 @@ class Parser : public AsyncWrap, public StreamListener {
442444

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

445-
MaybeLocal<Value> head_response;
446-
{
447-
InternalCallbackScope callback_scope(
448-
this, InternalCallbackScope::kSkipTaskQueues);
449-
head_response = cb.As<Function>()->Call(
450-
env()->context(), object(), arraysize(argv), argv);
451-
if (head_response.IsEmpty()) callback_scope.MarkAsFailed();
447+
v8::TryCatch try_catch(env()->isolate());
448+
auto head_response = cb.As<Function>()->Call(
449+
env()->context(), object(), arraysize(argv), argv);
450+
if (try_catch.HasCaught()) {
451+
got_exception_ = true;
452+
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
453+
return HPE_USER;
452454
}
453-
454455
int64_t val;
455456

456457
if (head_response.IsEmpty() || !head_response.ToLocalChecked()
@@ -478,9 +479,10 @@ class Parser : public AsyncWrap, public StreamListener {
478479

479480
Local<Value> buffer = Buffer::Copy(env, at, length).ToLocalChecked();
480481

481-
MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 1, &buffer);
482+
v8::TryCatch try_catch(env->isolate());
483+
USE(cb.As<Function>()->Call(env->context(), object(), 1, &buffer));
482484

483-
if (r.IsEmpty()) {
485+
if (try_catch.HasCaught()) {
484486
got_exception_ = true;
485487
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
486488
return HPE_USER;
@@ -516,15 +518,10 @@ class Parser : public AsyncWrap, public StreamListener {
516518
if (!cb->IsFunction())
517519
return 0;
518520

519-
MaybeLocal<Value> r;
520-
{
521-
InternalCallbackScope callback_scope(
522-
this, InternalCallbackScope::kSkipTaskQueues);
523-
r = cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
524-
if (r.IsEmpty()) callback_scope.MarkAsFailed();
525-
}
521+
v8::TryCatch try_catch(env()->isolate());
522+
USE(cb.As<Function>()->Call(env()->context(), object(), 0, nullptr));
526523

527-
if (r.IsEmpty()) {
524+
if (try_catch.HasCaught()) {
528525
got_exception_ = true;
529526
return -1;
530527
}
@@ -571,17 +568,6 @@ class Parser : public AsyncWrap, public StreamListener {
571568
delete parser;
572569
}
573570

574-
// TODO(@anonrig): Add V8 Fast API
575-
static void Free(const FunctionCallbackInfo<Value>& args) {
576-
Parser* parser;
577-
ASSIGN_OR_RETURN_UNWRAP(&parser, args.This());
578-
579-
// Since the Parser destructor isn't going to run the destroy() callbacks
580-
// it needs to be triggered manually.
581-
parser->EmitTraceEventDestroy();
582-
parser->EmitDestroy();
583-
}
584-
585571
// TODO(@anonrig): Add V8 Fast API
586572
static void Remove(const FunctionCallbackInfo<Value>& args) {
587573
Parser* parser;
@@ -639,25 +625,24 @@ class Parser : public AsyncWrap, public StreamListener {
639625
ConnectionsList* connectionsList = nullptr;
640626

641627
CHECK(args[0]->IsInt32());
642-
CHECK(args[1]->IsObject());
643628

644-
if (args.Length() > 2) {
645-
CHECK(args[2]->IsNumber());
629+
if (args.Length() > 1) {
630+
CHECK(args[1]->IsNumber());
646631
max_http_header_size =
647-
static_cast<uint64_t>(args[2].As<Number>()->Value());
632+
static_cast<uint64_t>(args[1].As<Number>()->Value());
648633
}
649634
if (max_http_header_size == 0) {
650635
max_http_header_size = env->options()->max_http_header_size;
651636
}
652637

653-
if (args.Length() > 3) {
654-
CHECK(args[3]->IsInt32());
655-
lenient_flags = args[3].As<Int32>()->Value();
638+
if (args.Length() > 2) {
639+
CHECK(args[2]->IsInt32());
640+
lenient_flags = args[2].As<Int32>()->Value();
656641
}
657642

658-
if (args.Length() > 4 && !args[4]->IsNullOrUndefined()) {
659-
CHECK(args[4]->IsObject());
660-
ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[4]);
643+
if (args.Length() > 3 && !args[3]->IsNullOrUndefined()) {
644+
CHECK(args[3]->IsObject());
645+
ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[3]);
661646
}
662647

663648
llhttp_type_t type =
@@ -669,13 +654,6 @@ class Parser : public AsyncWrap, public StreamListener {
669654
// Should always be called from the same context.
670655
CHECK_EQ(env, parser->env());
671656

672-
AsyncWrap::ProviderType provider =
673-
(type == HTTP_REQUEST ?
674-
AsyncWrap::PROVIDER_HTTPINCOMINGMESSAGE
675-
: AsyncWrap::PROVIDER_HTTPCLIENTREQUEST);
676-
677-
parser->set_provider_type(provider);
678-
parser->AsyncReset(args[1].As<Object>());
679657
parser->Init(type, max_http_header_size, lenient_flags);
680658

681659
if (connectionsList != nullptr) {
@@ -802,7 +780,14 @@ class Parser : public AsyncWrap, public StreamListener {
802780
current_buffer_len_ = nread;
803781
current_buffer_data_ = buf.base;
804782

805-
MakeCallback(cb.As<Function>(), 1, &ret);
783+
v8::TryCatch try_catch(env()->isolate());
784+
USE(cb.As<Function>()->Call(env()->context(), object(), 1, &ret));
785+
786+
if (try_catch.HasCaught()) {
787+
got_exception_ = true;
788+
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
789+
return;
790+
}
806791

807792
current_buffer_len_ = 0;
808793
current_buffer_data_ = nullptr;
@@ -917,12 +902,11 @@ class Parser : public AsyncWrap, public StreamListener {
917902
url_.ToString(env())
918903
};
919904

920-
MaybeLocal<Value> r = MakeCallback(cb.As<Function>(),
921-
arraysize(argv),
922-
argv);
905+
v8::TryCatch try_catch(env()->isolate());
906+
USE(cb.As<Function>()->Call(
907+
env()->context(), object(), arraysize(argv), argv));
923908

924-
if (r.IsEmpty())
925-
got_exception_ = true;
909+
if (try_catch.HasCaught()) got_exception_ = true;
926910

927911
url_.Reset();
928912
have_flushed_ = true;
@@ -1287,9 +1271,7 @@ void CreatePerIsolateProperties(IsolateData* isolate_data,
12871271
t->Set(FIXED_ONE_BYTE_STRING(isolate, "kLenientAll"),
12881272
Integer::NewFromUnsigned(isolate, kLenientAll));
12891273

1290-
t->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data));
12911274
SetProtoMethod(isolate, t, "close", Parser::Close);
1292-
SetProtoMethod(isolate, t, "free", Parser::Free);
12931275
SetProtoMethod(isolate, t, "remove", Parser::Remove);
12941276
SetProtoMethod(isolate, t, "execute", Parser::Execute);
12951277
SetProtoMethod(isolate, t, "finish", Parser::Finish);
@@ -1358,7 +1340,6 @@ void CreatePerContextProperties(Local<Object> target,
13581340
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
13591341
registry->Register(Parser::New);
13601342
registry->Register(Parser::Close);
1361-
registry->Register(Parser::Free);
13621343
registry->Register(Parser::Remove);
13631344
registry->Register(Parser::Execute);
13641345
registry->Register(Parser::Finish);

‎test/async-hooks/test-graph.http.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,7 @@ process.on('exit', () => {
3636
{ type: 'TCPCONNECTWRAP',
3737
id: 'tcpconnect:1',
3838
triggerAsyncId: 'tcp:1' },
39-
{ type: 'HTTPCLIENTREQUEST',
40-
id: 'httpclientrequest:1',
41-
triggerAsyncId: 'tcpserver:1' },
4239
{ type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' },
43-
{ type: 'HTTPINCOMINGMESSAGE',
44-
id: 'httpincomingmessage:1',
45-
triggerAsyncId: 'tcp:2' },
4640
{ type: 'Timeout',
4741
id: 'timeout:1',
4842
triggerAsyncId: null },

0 commit comments

Comments
 (0)