src: do not use std::function for OnScopeLeave

Using `std::function` adds an extra layer of indirection, and in particular, heap allocations that are not necessary in our use case here. PR-URL: #30134 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Nov 17, 2019 · 058a8d53631df7aba5fbbff8859ebc8e88fade49 · 058a8d5
1 parent 31a3b72
commit 058a8d53631df7aba5fbbff8859ebc8e88fade49
diff --git a/src/api/callback.cc b/src/api/callback.cc
@@ -105,7 +105,7 @@ void InternalCallbackScope::Close() {
 
   if (!env_->can_call_into_js()) return;
 
-  OnScopeLeave weakref_cleanup([&]() { env_->RunWeakRefCleanup(); });
+  auto weakref_cleanup = OnScopeLeave([&]() { env_->RunWeakRefCleanup(); });
 
   if (!tick_info->has_tick_scheduled()) {
     MicrotasksScope::PerformCheckpoint(env_->isolate());

diff --git a/src/large_pages/node_large_page.cc b/src/large_pages/node_large_page.cc
@@ -333,7 +333,7 @@ MoveTextRegionToLargePages(const text_region& r) {
     PrintSystemError(errno);
     return -1;
   }
-  OnScopeLeave munmap_on_return([nmem, size]() {
+  auto munmap_on_return = OnScopeLeave([nmem, size]() {
     if (-1 == munmap(nmem, size)) PrintSystemError(errno);
   });
 

diff --git a/src/node_binding.cc b/src/node_binding.cc
@@ -152,7 +152,7 @@ void* wrapped_dlopen(const char* filename, int flags) {
   Mutex::ScopedLock lock(dlhandles_mutex);
 
   uv_fs_t req;
-  OnScopeLeave cleanup([&]() { uv_fs_req_cleanup(&req); });
+  auto cleanup = OnScopeLeave([&]() { uv_fs_req_cleanup(&req); });
   int rc = uv_fs_stat(nullptr, &req, filename, nullptr);
 
   if (rc != 0) {

diff --git a/src/node_env_var.cc b/src/node_env_var.cc
@@ -149,7 +149,7 @@ Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
   uv_env_item_t* items;
   int count;
 
-  OnScopeLeave cleanup([&]() { uv_os_free_environ(items, count); });
+  auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); });
   CHECK_EQ(uv_os_environ(&items, &count), 0);
 
   MaybeStackBuffer<Local<Value>, 256> env_v(count);

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
@@ -586,7 +586,7 @@ class Parser : public AsyncWrap, public StreamListener {
     // Once we’re done here, either indicate that the HTTP parser buffer
     // is free for re-use, or free() the data if it didn’t come from there
     // in the first place.
-    OnScopeLeave on_scope_leave([&]() {
+    auto on_scope_leave = OnScopeLeave([&]() {
       if (buf.base == env()->http_parser_buffer())
         env()->set_http_parser_buffer_in_use(false);
       else

diff --git a/src/node_i18n.cc b/src/node_i18n.cc
@@ -217,7 +217,7 @@ class ConverterObject : public BaseObject, Converter {
       result.AllocateSufficientStorage(limit);
 
     UBool flush = (flags & CONVERTER_FLAGS_FLUSH) == CONVERTER_FLAGS_FLUSH;
-    OnScopeLeave cleanup([&]() {
+    auto cleanup = OnScopeLeave([&]() {
       if (flush) {
         // Reset the converter state.
         converter->bomSeen_ = false;

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -809,7 +809,7 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
   per_process::cli_options->per_isolate = env->isolate_data()->options();
   auto original_per_env = per_process::cli_options->per_isolate->per_env;
   per_process::cli_options->per_isolate->per_env = env->options();
-  OnScopeLeave on_scope_leave([&]() {
+  auto on_scope_leave = OnScopeLeave([&]() {
     per_process::cli_options->per_isolate->per_env = original_per_env;
     per_process::cli_options->per_isolate = original_per_isolate;
   });

diff --git a/src/node_os.cc b/src/node_os.cc
@@ -302,7 +302,7 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
     return args.GetReturnValue().SetUndefined();
   }
 
-  OnScopeLeave free_passwd([&]() { uv_os_free_passwd(&pwd); });
+  auto free_passwd = OnScopeLeave([&]() { uv_os_free_passwd(&pwd); });
 
   Local<Value> error;
 

diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc
@@ -362,7 +362,7 @@ static void DebugProcess(const FunctionCallbackInfo<Value>& args) {
   LPTHREAD_START_ROUTINE* handler = nullptr;
   DWORD pid = 0;
 
-  OnScopeLeave cleanup([&]() {
+  auto cleanup = OnScopeLeave([&]() {
     if (process != nullptr) CloseHandle(process);
     if (thread != nullptr) CloseHandle(thread);
     if (handler != nullptr) UnmapViewOfFile(handler);

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -242,7 +242,7 @@ void Worker::Run() {
     SealHandleScope outer_seal(isolate_);
 
     DeleteFnPtr<Environment, FreeEnvironment> env_;
-    OnScopeLeave cleanup_env([&]() {
+    auto cleanup_env = OnScopeLeave([&]() {
       if (!env_) return;
       env_->set_can_call_into_js(false);
       Isolate::DisallowJavascriptExecutionScope disallow_js(isolate_,

diff --git a/src/node_zlib.cc b/src/node_zlib.cc
@@ -386,7 +386,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
   // v8 land!
   void AfterThreadPoolWork(int status) override {
     AllocScope alloc_scope(this);
-    OnScopeLeave on_scope_leave([&]() { Unref(); });
+    auto on_scope_leave = OnScopeLeave([&]() { Unref(); });
 
     write_in_progress_ = false;
 

diff --git a/src/util.h b/src/util.h
@@ -42,6 +42,12 @@
 #include <unordered_map>
 #include <utility>
 
+#ifdef __GNUC__
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#else
+#define MUST_USE_RESULT
+#endif
+
 namespace node {
 
 // Maybe remove kPathSeparator when cpp17 is ready
@@ -494,14 +500,37 @@ class BufferValue : public MaybeStackBuffer<char> {
 // silence a compiler warning about that.
 template <typename T> inline void USE(T&&) {}
 
-// Run a function when exiting the current scope.
-struct OnScopeLeave {
-  std::function<void()> fn_;
+template <typename Fn>
+struct OnScopeLeaveImpl {
+  Fn fn_;
+  bool active_;
+
+  explicit OnScopeLeaveImpl(Fn&& fn) : fn_(std::move(fn)), active_(true) {}
+  ~OnScopeLeaveImpl() { if (active_) fn_(); }
 
-  explicit OnScopeLeave(std::function<void()> fn) : fn_(std::move(fn)) {}
-  ~OnScopeLeave() { fn_(); }
+  OnScopeLeaveImpl(const OnScopeLeaveImpl& other) = delete;
+  OnScopeLeaveImpl& operator=(const OnScopeLeaveImpl& other) = delete;
+  OnScopeLeaveImpl(OnScopeLeaveImpl&& other)
+    : fn_(std::move(other.fn_)), active_(other.active_) {
+    other.active_ = false;
+  }
+  OnScopeLeaveImpl& operator=(OnScopeLeaveImpl&& other) {
+    if (this == &other) return *this;
+    this->~OnScopeLeave();
+    new (this)OnScopeLeaveImpl(std::move(other));
+    return *this;
+  }
 };
 
+// Run a function when exiting the current scope. Used like this:
+// auto on_scope_leave = OnScopeLeave([&] {
+//   // ... run some code ...
+// });
+template <typename Fn>
+inline MUST_USE_RESULT OnScopeLeaveImpl<Fn> OnScopeLeave(Fn&& fn) {
+  return OnScopeLeaveImpl<Fn>{std::move(fn)};
+}
+
 // Simple RAII wrapper for contiguous data that uses malloc()/free().
 template <typename T>
 struct MallocedBuffer {
@@ -679,12 +708,6 @@ constexpr T RoundUp(T a, T b) {
   return a % b != 0 ? a + b - (a % b) : a;
 }
 
-#ifdef __GNUC__
-#define MUST_USE_RESULT __attribute__((warn_unused_result))
-#else
-#define MUST_USE_RESULT
-#endif
-
 class SlicedArguments : public MaybeStackBuffer<v8::Local<v8::Value>> {
  public:
   inline explicit SlicedArguments(