Skip to content

Commit ddce8fa

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

20 files changed

+86
-505
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: 41 additions & 68 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,19 @@ class Parser : public AsyncWrap, public StreamListener {
286283
connectionsList_->PushActive(this);
287284
}
288285

289-
Local<Value> cb = object()->Get(env()->context(), kOnMessageBegin)
286+
auto context = env()->context();
287+
288+
Local<Value> cb = object()->Get(context, kOnMessageBegin)
290289
.ToLocalChecked();
291290
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);
291+
v8::TryCatch try_catch(env()->isolate());
292+
USE(cb.As<Function>()->Call(context, object(), 0, nullptr));
297293

298-
if (r.IsEmpty()) callback_scope.MarkAsFailed();
294+
if (try_catch.HasCaught()) {
295+
got_exception_ = true;
296+
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
297+
return HPE_USER;
298+
}
299299
}
300300

301301
return 0;
@@ -442,14 +442,8 @@ class Parser : public AsyncWrap, public StreamListener {
442442

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

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();
452-
}
445+
auto head_response = cb.As<Function>()->Call(
446+
env()->context(), object(), arraysize(argv), argv);
453447

454448
int64_t val;
455449

@@ -478,9 +472,10 @@ class Parser : public AsyncWrap, public StreamListener {
478472

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

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

483-
if (r.IsEmpty()) {
478+
if (try_catch.HasCaught()) {
484479
got_exception_ = true;
485480
llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
486481
return HPE_USER;
@@ -516,15 +511,10 @@ class Parser : public AsyncWrap, public StreamListener {
516511
if (!cb->IsFunction())
517512
return 0;
518513

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

527-
if (r.IsEmpty()) {
517+
if (try_catch.HasCaught()) {
528518
got_exception_ = true;
529519
return -1;
530520
}
@@ -571,17 +561,6 @@ class Parser : public AsyncWrap, public StreamListener {
571561
delete parser;
572562
}
573563

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-
585564
// TODO(@anonrig): Add V8 Fast API
586565
static void Remove(const FunctionCallbackInfo<Value>& args) {
587566
Parser* parser;
@@ -639,25 +618,24 @@ class Parser : public AsyncWrap, public StreamListener {
639618
ConnectionsList* connectionsList = nullptr;
640619

641620
CHECK(args[0]->IsInt32());
642-
CHECK(args[1]->IsObject());
643621

644-
if (args.Length() > 2) {
645-
CHECK(args[2]->IsNumber());
622+
if (args.Length() > 1) {
623+
CHECK(args[1]->IsNumber());
646624
max_http_header_size =
647-
static_cast<uint64_t>(args[2].As<Number>()->Value());
625+
static_cast<uint64_t>(args[1].As<Number>()->Value());
648626
}
649627
if (max_http_header_size == 0) {
650628
max_http_header_size = env->options()->max_http_header_size;
651629
}
652630

653-
if (args.Length() > 3) {
654-
CHECK(args[3]->IsInt32());
655-
lenient_flags = args[3].As<Int32>()->Value();
631+
if (args.Length() > 2) {
632+
CHECK(args[2]->IsInt32());
633+
lenient_flags = args[2].As<Int32>()->Value();
656634
}
657635

658-
if (args.Length() > 4 && !args[4]->IsNullOrUndefined()) {
659-
CHECK(args[4]->IsObject());
660-
ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[4]);
636+
if (args.Length() > 3 && !args[3]->IsNullOrUndefined()) {
637+
CHECK(args[3]->IsObject());
638+
ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[3]);
661639
}
662640

663641
llhttp_type_t type =
@@ -669,13 +647,6 @@ class Parser : public AsyncWrap, public StreamListener {
669647
// Should always be called from the same context.
670648
CHECK_EQ(env, parser->env());
671649

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

681652
if (connectionsList != nullptr) {
@@ -802,7 +773,13 @@ class Parser : public AsyncWrap, public StreamListener {
802773
current_buffer_len_ = nread;
803774
current_buffer_data_ = buf.base;
804775

805-
MakeCallback(cb.As<Function>(), 1, &ret);
776+
v8::TryCatch try_catch(env()->isolate());
777+
USE(cb.As<Function>()->Call(env()->context(), object(), 1, &ret));
778+
779+
if (try_catch.HasCaught()) {
780+
got_exception_ = true;
781+
return;
782+
}
806783

807784
current_buffer_len_ = 0;
808785
current_buffer_data_ = nullptr;
@@ -917,12 +894,11 @@ class Parser : public AsyncWrap, public StreamListener {
917894
url_.ToString(env())
918895
};
919896

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

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

927903
url_.Reset();
928904
have_flushed_ = true;
@@ -1287,9 +1263,7 @@ void CreatePerIsolateProperties(IsolateData* isolate_data,
12871263
t->Set(FIXED_ONE_BYTE_STRING(isolate, "kLenientAll"),
12881264
Integer::NewFromUnsigned(isolate, kLenientAll));
12891265

1290-
t->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data));
12911266
SetProtoMethod(isolate, t, "close", Parser::Close);
1292-
SetProtoMethod(isolate, t, "free", Parser::Free);
12931267
SetProtoMethod(isolate, t, "remove", Parser::Remove);
12941268
SetProtoMethod(isolate, t, "execute", Parser::Execute);
12951269
SetProtoMethod(isolate, t, "finish", Parser::Finish);
@@ -1358,7 +1332,6 @@ void CreatePerContextProperties(Local<Object> target,
13581332
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
13591333
registry->Register(Parser::New);
13601334
registry->Register(Parser::Close);
1361-
registry->Register(Parser::Free);
13621335
registry->Register(Parser::Remove);
13631336
registry->Register(Parser::Execute);
13641337
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)