Skip to content

Commit 0e7a62e

Browse files
anonrigJonas Badalic
anonrig
and
Jonas Badalic
committed
http: remove async tracking
Co-Authored-by: Jonas Badalic <[email protected]>
1 parent 255dd7b commit 0e7a62e

20 files changed

+95
-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: 50 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include "node_buffer.h"
2424
#include "util.h"
2525

26-
#include "async_wrap-inl.h"
2726
#include "env-inl.h"
2827
#include "llhttp.h"
2928
#include "memory_tracker-inl.h"
@@ -64,7 +63,6 @@ using v8::Integer;
6463
using v8::Isolate;
6564
using v8::Local;
6665
using v8::LocalVector;
67-
using v8::MaybeLocal;
6866
using v8::Number;
6967
using v8::Object;
7068
using v8::ObjectTemplate;
@@ -250,17 +248,16 @@ class ConnectionsList : public BaseObject {
250248
std::set<Parser*, ParserComparator> active_connections_;
251249
};
252250

253-
class Parser : public AsyncWrap, public StreamListener {
251+
class Parser : public BaseObject, public StreamListener {
254252
friend class ConnectionsList;
255253
friend struct ParserComparator;
256254

257255
public:
258256
Parser(BindingData* binding_data, Local<Object> wrap)
259-
: AsyncWrap(binding_data->env(), wrap),
257+
: BaseObject(binding_data->env(), wrap),
260258
current_buffer_len_(0),
261259
current_buffer_data_(nullptr),
262-
binding_data_(binding_data) {
263-
}
260+
binding_data_(binding_data) {}
264261

265262
SET_NO_MEMORY_INFO()
266263
SET_MEMORY_INFO_NAME(Parser)
@@ -286,16 +283,20 @@ class Parser : public AsyncWrap, public StreamListener {
286283
connectionsList_->PushActive(this);
287284
}
288285

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);
286+
auto context = env()->context();
297287

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

301302
return 0;
@@ -442,15 +443,14 @@ class Parser : public AsyncWrap, public StreamListener {
442443

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

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();
446+
v8::TryCatch try_catch(env()->isolate());
447+
auto head_response = cb.As<Function>()->Call(
448+
env()->context(), object(), arraysize(argv), argv);
449+
if (try_catch.HasCaught()) {
450+
got_exception_ = true;
451+
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
452+
return HPE_USER;
452453
}
453-
454454
int64_t val;
455455

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

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

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

483-
if (r.IsEmpty()) {
484+
if (try_catch.HasCaught()) {
484485
got_exception_ = true;
485486
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
486487
return HPE_USER;
@@ -516,15 +517,10 @@ class Parser : public AsyncWrap, public StreamListener {
516517
if (!cb->IsFunction())
517518
return 0;
518519

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-
}
520+
v8::TryCatch try_catch(env()->isolate());
521+
USE(cb.As<Function>()->Call(env()->context(), object(), 0, nullptr));
526522

527-
if (r.IsEmpty()) {
523+
if (try_catch.HasCaught()) {
528524
got_exception_ = true;
529525
return -1;
530526
}
@@ -571,17 +567,6 @@ class Parser : public AsyncWrap, public StreamListener {
571567
delete parser;
572568
}
573569

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-
585570
// TODO(@anonrig): Add V8 Fast API
586571
static void Remove(const FunctionCallbackInfo<Value>& args) {
587572
Parser* parser;
@@ -639,25 +624,24 @@ class Parser : public AsyncWrap, public StreamListener {
639624
ConnectionsList* connectionsList = nullptr;
640625

641626
CHECK(args[0]->IsInt32());
642-
CHECK(args[1]->IsObject());
643627

644-
if (args.Length() > 2) {
645-
CHECK(args[2]->IsNumber());
628+
if (args.Length() > 1) {
629+
CHECK(args[1]->IsNumber());
646630
max_http_header_size =
647-
static_cast<uint64_t>(args[2].As<Number>()->Value());
631+
static_cast<uint64_t>(args[1].As<Number>()->Value());
648632
}
649633
if (max_http_header_size == 0) {
650634
max_http_header_size = env->options()->max_http_header_size;
651635
}
652636

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

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

663647
llhttp_type_t type =
@@ -669,13 +653,6 @@ class Parser : public AsyncWrap, public StreamListener {
669653
// Should always be called from the same context.
670654
CHECK_EQ(env, parser->env());
671655

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>());
679656
parser->Init(type, max_http_header_size, lenient_flags);
680657

681658
if (connectionsList != nullptr) {
@@ -802,7 +779,14 @@ class Parser : public AsyncWrap, public StreamListener {
802779
current_buffer_len_ = nread;
803780
current_buffer_data_ = buf.base;
804781

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

807791
current_buffer_len_ = 0;
808792
current_buffer_data_ = nullptr;
@@ -917,12 +901,11 @@ class Parser : public AsyncWrap, public StreamListener {
917901
url_.ToString(env())
918902
};
919903

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

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

927910
url_.Reset();
928911
have_flushed_ = true;
@@ -1287,9 +1270,7 @@ void CreatePerIsolateProperties(IsolateData* isolate_data,
12871270
t->Set(FIXED_ONE_BYTE_STRING(isolate, "kLenientAll"),
12881271
Integer::NewFromUnsigned(isolate, kLenientAll));
12891272

1290-
t->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data));
12911273
SetProtoMethod(isolate, t, "close", Parser::Close);
1292-
SetProtoMethod(isolate, t, "free", Parser::Free);
12931274
SetProtoMethod(isolate, t, "remove", Parser::Remove);
12941275
SetProtoMethod(isolate, t, "execute", Parser::Execute);
12951276
SetProtoMethod(isolate, t, "finish", Parser::Finish);
@@ -1358,7 +1339,6 @@ void CreatePerContextProperties(Local<Object> target,
13581339
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
13591340
registry->Register(Parser::New);
13601341
registry->Register(Parser::Close);
1361-
registry->Register(Parser::Free);
13621342
registry->Register(Parser::Remove);
13631343
registry->Register(Parser::Execute);
13641344
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)