FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

domain: further abstract usage in C++ · nodejs/node@44bf0f4 · GitHub

/ node Public

Commit 44bf0f4

Browse files
apapirovski
authored and
MylesBorins
committed
domain: further abstract usage in C++
Move the majority of C++ domain-related code into JS land by introducing a top level domain callback which handles entering & exiting the domain. Move the rest of the domain necessities into their own file that creates an internal binding, to avoid exposing domain-related code on the process object. Modify an existing test slightly to better test domain-related code. PR-URL: #18291 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent d764039 commit 44bf0f4

10 files changed

Lines changed: 72 additions & 90 deletions

File tree

‎lib/domain.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,21 @@ process.setUncaughtExceptionCaptureCallback = function(fn) {
9494
throw err;
9595
};
9696

97+
function topLevelDomainCallback(cb, ...args) {
98+
const domain = this.domain;
99+
if (domain)
100+
domain.enter();
101+
const ret = Reflect.apply(cb, this, args);
102+
if (domain)
103+
domain.exit();
104+
return ret;
105+
}
106+
97107
// It's possible to enter one domain while already inside
98108
// another one. The stack is each entered domain.
99109
const stack = [];
100110
exports._stack = stack;
101-
process._setupDomainUse();
111+
internalBinding('domain').enable(topLevelDomainCallback);
102112

103113
function updateExceptionCapture() {
104114
if (stack.every((domain) => domain.listenerCount('error') === 0)) {

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@
293293
'src/node_constants.cc',
294294
'src/node_contextify.cc',
295295
'src/node_debug_options.cc',
296+
'src/node_domain.cc',
296297
'src/node_file.cc',
297298
'src/node_http2.cc',
298299
'src/node_http_parser.cc',

‎src/env-inl.h‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,6 @@ inline Environment::Environment(IsolateData* isolate_data,
278278
: isolate_(context->GetIsolate()),
279279
isolate_data_(isolate_data),
280280
timer_base_(uv_now(isolate_data->event_loop())),
281-
using_domains_(false),
282281
printed_error_(false),
283282
trace_sync_io_(false),
284283
abort_on_uncaught_exception_(false),
@@ -379,14 +378,6 @@ inline uint64_t Environment::timer_base() const {
379378
return timer_base_;
380379
}
381380

382-
inline bool Environment::using_domains() const {
383-
return using_domains_;
384-
}
385-
386-
inline void Environment::set_using_domains(bool value) {
387-
using_domains_ = value;
388-
}
389-
390381
inline bool Environment::printed_error() const {
391382
return printed_error_;
392383
}

‎src/env.h‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ class ModuleWrap;
9191
V(decorated_private_symbol, "node:decorated") \
9292
V(npn_buffer_private_symbol, "node:npnBuffer") \
9393
V(selected_npn_buffer_private_symbol, "node:selectedNpnBuffer") \
94-
V(domain_private_symbol, "node:domain") \
9594

9695
// Strings are per-isolate primitives but Environment proxies them
9796
// for the sake of convenience. Strings should be ASCII-only.
@@ -128,7 +127,6 @@ class ModuleWrap;
128127
V(dns_soa_string, "SOA") \
129128
V(dns_srv_string, "SRV") \
130129
V(dns_txt_string, "TXT") \
131-
V(domain_string, "domain") \
132130
V(emit_warning_string, "emitWarning") \
133131
V(exchange_string, "exchange") \
134132
V(encoding_string, "encoding") \
@@ -283,6 +281,7 @@ class ModuleWrap;
283281
V(async_hooks_binding, v8::Object) \
284282
V(buffer_prototype_object, v8::Object) \
285283
V(context, v8::Context) \
284+
V(domain_callback, v8::Function) \
286285
V(host_import_module_dynamically_callback, v8::Function) \
287286
V(http2ping_constructor_template, v8::ObjectTemplate) \
288287
V(http2stream_constructor_template, v8::ObjectTemplate) \
@@ -537,9 +536,6 @@ class Environment {
537536

538537
inline IsolateData* isolate_data() const;
539538

540-
inline bool using_domains() const;
541-
inline void set_using_domains(bool value);
542-
543539
inline bool printed_error() const;
544540
inline void set_printed_error(bool value);
545541

@@ -693,7 +689,6 @@ class Environment {
693689
AsyncHooks async_hooks_;
694690
TickInfo tick_info_;
695691
const uint64_t timer_base_;
696-
bool using_domains_;
697692
bool printed_error_;
698693
bool trace_sync_io_;
699694
bool abort_on_uncaught_exception_;

‎src/node.cc‎

Lines changed: 10 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,62 +1118,6 @@ bool ShouldAbortOnUncaughtException(Isolate* isolate) {
11181118
}
11191119

11201120

1121-
Local<Value> GetDomainProperty(Environment* env, Local<Object> object) {
1122-
Local<Value> domain_v =
1123-
object->GetPrivate(env->context(), env->domain_private_symbol())
1124-
.ToLocalChecked();
1125-
if (domain_v->IsObject()) {
1126-
return domain_v;
1127-
}
1128-
return object->Get(env->context(), env->domain_string()).ToLocalChecked();
1129-
}
1130-
1131-
1132-
void DomainEnter(Environment* env, Local<Object> object) {
1133-
Local<Value> domain_v = GetDomainProperty(env, object);
1134-
if (domain_v->IsObject()) {
1135-
Local<Object> domain = domain_v.As<Object>();
1136-
Local<Value> enter_v = domain->Get(env->enter_string());
1137-
if (enter_v->IsFunction()) {
1138-
if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
1139-
FatalError("node::AsyncWrap::MakeCallback",
1140-
"domain enter callback threw, please report this");
1141-
}
1142-
}
1143-
}
1144-
}
1145-
1146-
1147-
void DomainExit(Environment* env, v8::Local<v8::Object> object) {
1148-
Local<Value> domain_v = GetDomainProperty(env, object);
1149-
if (domain_v->IsObject()) {
1150-
Local<Object> domain = domain_v.As<Object>();
1151-
Local<Value> exit_v = domain->Get(env->exit_string());
1152-
if (exit_v->IsFunction()) {
1153-
if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
1154-
FatalError("node::AsyncWrap::MakeCallback",
1155-
"domain exit callback threw, please report this");
1156-
}
1157-
}
1158-
}
1159-
}
1160-
1161-
void SetupDomainUse(const FunctionCallbackInfo<Value>& args) {
1162-
Environment* env = Environment::GetCurrent(args);
1163-
1164-
if (env->using_domains())
1165-
return;
1166-
env->set_using_domains(true);
1167-
1168-
HandleScope scope(env->isolate());
1169-
1170-
// Do a little housekeeping.
1171-
env->process_object()->Delete(
1172-
env->context(),
1173-
FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_setupDomainUse")).FromJust();
1174-
}
1175-
1176-
11771121
void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
11781122
args.GetIsolate()->RunMicrotasks();
11791123
}
@@ -1294,11 +1238,6 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
12941238
// If you hit this assertion, you forgot to enter the v8::Context first.
12951239
CHECK_EQ(Environment::GetCurrent(env->isolate()), env);
12961240

1297-
if (asyncContext.async_id == 0 && env->using_domains() &&
1298-
!object_.IsEmpty()) {
1299-
DomainEnter(env, object_);
1300-
}
1301-
13021241
if (asyncContext.async_id != 0) {
13031242
// No need to check a return value because the application will exit if
13041243
// an exception occurs.
@@ -1328,11 +1267,6 @@ void InternalCallbackScope::Close() {
13281267
AsyncWrap::EmitAfter(env_, async_context_.async_id);
13291268
}
13301269

1331-
if (async_context_.async_id == 0 && env_->using_domains() &&
1332-
!object_.IsEmpty()) {
1333-
DomainExit(env_, object_);
1334-
}
1335-
13361270
if (IsInnerMakeCallback()) {
13371271
return;
13381272
}
@@ -1379,7 +1313,16 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env,
13791313
return Undefined(env->isolate());
13801314
}
13811315

1382-
MaybeLocal<Value> ret = callback->Call(env->context(), recv, argc, argv);
1316+
Local<Function> domain_cb = env->domain_callback();
1317+
MaybeLocal<Value> ret;
1318+
if (asyncContext.async_id != 0 || domain_cb.IsEmpty() || recv.IsEmpty()) {
1319+
ret = callback->Call(env->context(), recv, argc, argv);
1320+
} else {
1321+
std::vector<Local<Value>> args(1 + argc);
1322+
args[0] = callback;
1323+
std::copy(&argv[0], &argv[argc], &args[1]);
1324+
ret = domain_cb->Call(env->context(), recv, args.size(), &args[0]);
1325+
}
13831326

13841327
if (ret.IsEmpty()) {
13851328
// NOTE: For backwards compatibility with public API we return Undefined()
@@ -3635,7 +3578,6 @@ void SetupProcessObject(Environment* env,
36353578
env->SetMethod(process, "_setupProcessObject", SetupProcessObject);
36363579
env->SetMethod(process, "_setupNextTick", SetupNextTick);
36373580
env->SetMethod(process, "_setupPromises", SetupPromises);
3638-
env->SetMethod(process, "_setupDomainUse", SetupDomainUse);
36393581
}
36403582

36413583

‎src/node_domain.cc‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
#include "v8.h"
2+
#include "node_internals.h"
3+
4+
namespace node {
5+
namespace domain {
6+
7+
using v8::Context;
8+
using v8::Function;
9+
using v8::FunctionCallbackInfo;
10+
using v8::Local;
11+
using v8::Object;
12+
using v8::Value;
13+
14+
15+
void Enable(const FunctionCallbackInfo<Value>& args) {
16+
Environment* env = Environment::GetCurrent(args);
17+
18+
CHECK(args[0]->IsFunction());
19+
20+
env->set_domain_callback(args[0].As<Function>());
21+
}
22+
23+
void Initialize(Local<Object> target,
24+
Local<Value> unused,
25+
Local<Context> context) {
26+
Environment* env = Environment::GetCurrent(context);
27+
28+
env->SetMethod(target, "enable", Enable);
29+
}
30+
31+
} // namespace domain
32+
} // namespace node
33+
34+
NODE_MODULE_CONTEXT_AWARE_INTERNAL(domain, node::domain::Initialize)

‎src/node_internals.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ struct sockaddr;
104104
V(cares_wrap) \
105105
V(config) \
106106
V(contextify) \
107+
V(domain) \
107108
V(fs) \
108109
V(fs_event_wrap) \
109110
V(http2) \

‎test/addons/repl-domain-abort/binding.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <node.h>
2323
#include <v8.h>
2424

25+
using v8::Boolean;
2526
using v8::Function;
2627
using v8::FunctionCallbackInfo;
2728
using v8::Local;
@@ -31,11 +32,16 @@ using v8::Value;
3132

3233
void Method(const FunctionCallbackInfo<Value>& args) {
3334
Isolate* isolate = args.GetIsolate();
34-
node::MakeCallback(isolate,
35-
isolate->GetCurrentContext()->Global(),
36-
args[0].As<Function>(),
37-
0,
38-
nullptr);
35+
Local<Value> params[] = {
36+
Boolean::New(isolate, true),
37+
Boolean::New(isolate, false)
38+
};
39+
Local<Value> ret = node::MakeCallback(isolate,
40+
isolate->GetCurrentContext()->Global(),
41+
args[0].As<Function>(),
42+
2,
43+
params);
44+
assert(ret->IsTrue());
3945
}
4046

4147
void init(Local<Object> exports) {

‎test/addons/repl-domain-abort/test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ const lines = [
4040
// This line shouldn't cause an assertion error.
4141
`require('${buildPath}')` +
4242
// Log output to double check callback ran.
43-
'.method(function() { console.log(\'cb_ran\'); });',
43+
'.method(function(v1, v2) {' +
44+
'console.log(\'cb_ran\'); return v1 === true && v2 === false; });',
4445
];
4546

4647
const dInput = new stream.Readable();

‎test/cctest/node_module_reg.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
void _register_cares_wrap() {}
66
void _register_config() {}
77
void _register_contextify() {}
8+
void _register_domain() {}
89
void _register_fs() {}
910
void _register_fs_event_wrap() {}
1011
void _register_http2() {}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL