Repository: nodejs/node
Pull request: 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.)

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: src/module_wrap.cc
patch: @@ -757,8 +757,15 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
   bool timed_out = false;
   bool received_signal = false;
   MaybeLocal<Value> result;
-  auto run = [&]() {
-    MaybeLocal<Value> result = module->Evaluate(context);
+  {
+    auto wd = timeout != -1
+                  ? std::make_optional<Watchdog>(isolate, timeout, &timed_out)
+                  : std::nullopt;
+    auto swd = break_on_sigint ? std::make_optional<SigintWatchdog>(
+                                     isolate, &received_signal)
+                               : std::nullopt;
+
+    result = module->Evaluate(context);
 
     Local<Value> res;
     if (result.ToLocal(&res) && microtask_queue) {
@@ -792,29 +799,15 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
       Local<Context> outer_context = isolate->GetCurrentContext();
       Local<Promise::Resolver> resolver;
       if (!Promise::Resolver::New(outer_context).ToLocal(&resolver)) {
-        return MaybeLocal<Value>();
+        result = {};
       }
       if (resolver->Resolve(outer_context, res).IsNothing()) {
-        return MaybeLocal<Value>();
+        result = {};
       }
       result = resolver->GetPromise();
 
       microtask_queue->PerformCheckpoint(isolate);
     }
-    return result;
-  };
-  if (break_on_sigint && timeout != -1) {
-    Watchdog wd(isolate, timeout, &timed_out);
-    SigintWatchdog swd(isolate, &received_signal);
-    result = run();
-  } else if (break_on_sigint) {
-    SigintWatchdog swd(isolate, &received_signal);
-    result = run();
-  } else if (timeout != -1) {
-    Watchdog wd(isolate, timeout, &timed_out);
-    result = run();
-  } else {
-    result = run();
   }
 
   if (result.IsEmpty()) {

filename: src/node_contextify.cc
patch: @@ -1296,24 +1296,17 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
   MaybeLocal<Value> result;
   bool timed_out = false;
   bool received_signal = false;
-  auto run = [&]() {
-    MaybeLocal<Value> result = script->Run(context);
+  {
+    auto wd = timeout != -1 ? std::make_optional<Watchdog>(
+                                  env->isolate(), timeout, &timed_out)
+                            : std::nullopt;
+    auto swd = break_on_sigint ? std::make_optional<SigintWatchdog>(
+                                     env->isolate(), &received_signal)
+                               : std::nullopt;
+
+    result = script->Run(context);
     if (!result.IsEmpty() && mtask_queue != nullptr)
       mtask_queue->PerformCheckpoint(env->isolate());
-    return result;
-  };
-  if (break_on_sigint && timeout != -1) {
-    Watchdog wd(env->isolate(), timeout, &timed_out);
-    SigintWatchdog swd(env->isolate(), &received_signal);
-    result = run();
-  } else if (break_on_sigint) {
-    SigintWatchdog swd(env->isolate(), &received_signal);
-    result = run();
-  } else if (timeout != -1) {
-    Watchdog wd(env->isolate(), timeout, &timed_out);
-    result = run();
-  } else {
-    result = run();
   }
 
   // Convert the termination exception into a regular exception.

filename: src/node_watchdog.h
patch: @@ -51,6 +51,11 @@ class Watchdog {
   v8::Isolate* isolate() { return isolate_; }
 
  private:
+  Watchdog(const Watchdog&) = delete;
+  Watchdog& operator=(const Watchdog&) = delete;
+  Watchdog(Watchdog&&) = delete;
+  Watchdog& operator=(Watchdog&&) = delete;
+
   static void Run(void* arg);
   static void Timer(uv_timer_t* timer);
 
@@ -77,6 +82,11 @@ class SigintWatchdog : public SigintWatchdogBase {
   SignalPropagation HandleSigint() override;
 
  private:
+  SigintWatchdog(const SigintWatchdog&) = delete;
+  SigintWatchdog& operator=(const SigintWatchdog&) = delete;
+  SigintWatchdog(SigintWatchdog&&) = delete;
+  SigintWatchdog& operator=(SigintWatchdog&&) = delete;
+
   v8::Isolate* isolate_;
   bool* received_signal_;
 };