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

src: simplify watchdog instantiations via `std::optional` · nodejs/node@f14ed5a · GitHub

/ node Public

Commit f14ed5a

Browse files
addaleax
authored and
targos
committed
src: simplify watchdog instantiations via std::optional
It's been a bit of a code smell that we create these objects in different conditional branches, effectively forcing ourselves to duplicate logic between those branches. This code predates `std::optional` being available to us, but now that it is, it's the idiomatic way to resolve this issue. (This commit also explicitly deletes move and copy members for these classes; these had previously been omitted for brevity, but adding them made me feel more confident that there is no hidden copy operation added through this change.) PR-URL: #59960 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 49747a5 commit f14ed5a

3 files changed

Lines changed: 30 additions & 34 deletions

File tree

‎src/module_wrap.cc‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -757,8 +757,15 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
757757
bool timed_out = false;
758758
bool received_signal = false;
759759
MaybeLocal<Value> result;
760-
auto run = [&]() {
761-
MaybeLocal<Value> result = module->Evaluate(context);
760+
{
761+
auto wd = timeout != -1
762+
? std::make_optional<Watchdog>(isolate, timeout, &timed_out)
763+
: std::nullopt;
764+
auto swd = break_on_sigint ? std::make_optional<SigintWatchdog>(
765+
isolate, &received_signal)
766+
: std::nullopt;
767+
768+
result = module->Evaluate(context);
762769

763770
Local<Value> res;
764771
if (result.ToLocal(&res) && microtask_queue) {
@@ -792,29 +799,15 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
792799
Local<Context> outer_context = isolate->GetCurrentContext();
793800
Local<Promise::Resolver> resolver;
794801
if (!Promise::Resolver::New(outer_context).ToLocal(&resolver)) {
795-
return MaybeLocal<Value>();
802+
result = {};
796803
}
797804
if (resolver->Resolve(outer_context, res).IsNothing()) {
798-
return MaybeLocal<Value>();
805+
result = {};
799806
}
800807
result = resolver->GetPromise();
801808

802809
microtask_queue->PerformCheckpoint(isolate);
803810
}
804-
return result;
805-
};
806-
if (break_on_sigint && timeout != -1) {
807-
Watchdog wd(isolate, timeout, &timed_out);
808-
SigintWatchdog swd(isolate, &received_signal);
809-
result = run();
810-
} else if (break_on_sigint) {
811-
SigintWatchdog swd(isolate, &received_signal);
812-
result = run();
813-
} else if (timeout != -1) {
814-
Watchdog wd(isolate, timeout, &timed_out);
815-
result = run();
816-
} else {
817-
result = run();
818811
}
819812

820813
if (result.IsEmpty()) {

‎src/node_contextify.cc‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,24 +1296,17 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
12961296
MaybeLocal<Value> result;
12971297
bool timed_out = false;
12981298
bool received_signal = false;
1299-
auto run = [&]() {
1300-
MaybeLocal<Value> result = script->Run(context);
1299+
{
1300+
auto wd = timeout != -1 ? std::make_optional<Watchdog>(
1301+
env->isolate(), timeout, &timed_out)
1302+
: std::nullopt;
1303+
auto swd = break_on_sigint ? std::make_optional<SigintWatchdog>(
1304+
env->isolate(), &received_signal)
1305+
: std::nullopt;
1306+
1307+
result = script->Run(context);
13011308
if (!result.IsEmpty() && mtask_queue != nullptr)
13021309
mtask_queue->PerformCheckpoint(env->isolate());
1303-
return result;
1304-
};
1305-
if (break_on_sigint && timeout != -1) {
1306-
Watchdog wd(env->isolate(), timeout, &timed_out);
1307-
SigintWatchdog swd(env->isolate(), &received_signal);
1308-
result = run();
1309-
} else if (break_on_sigint) {
1310-
SigintWatchdog swd(env->isolate(), &received_signal);
1311-
result = run();
1312-
} else if (timeout != -1) {
1313-
Watchdog wd(env->isolate(), timeout, &timed_out);
1314-
result = run();
1315-
} else {
1316-
result = run();
13171310
}
13181311

13191312
// Convert the termination exception into a regular exception.

‎src/node_watchdog.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ class Watchdog {
5151
v8::Isolate* isolate() { return isolate_; }
5252

5353
private:
54+
Watchdog(const Watchdog&) = delete;
55+
Watchdog& operator=(const Watchdog&) = delete;
56+
Watchdog(Watchdog&&) = delete;
57+
Watchdog& operator=(Watchdog&&) = delete;
58+
5459
static void Run(void* arg);
5560
static void Timer(uv_timer_t* timer);
5661

@@ -77,6 +82,11 @@ class SigintWatchdog : public SigintWatchdogBase {
7782
SignalPropagation HandleSigint() override;
7883

7984
private:
85+
SigintWatchdog(const SigintWatchdog&) = delete;
86+
SigintWatchdog& operator=(const SigintWatchdog&) = delete;
87+
SigintWatchdog(SigintWatchdog&&) = delete;
88+
SigintWatchdog& operator=(SigintWatchdog&&) = delete;
89+
8090
v8::Isolate* isolate_;
8191
bool* received_signal_;
8292
};

0 commit comments

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