diff --git a/src/async_wrap.cc b/src/async_wrap.cc index b7c34f0121760c..580624afd90579 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -657,7 +657,7 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) { } if (env->destroy_async_id_list()->empty()) { - env->SetUnrefImmediate(&DestroyAsyncIdsCallback); + env->SetImmediate(&DestroyAsyncIdsCallback, CallbackFlags::kUnrefed); } env->destroy_async_id_list()->push_back(async_id); diff --git a/src/callback_queue-inl.h b/src/callback_queue-inl.h index e83c81cd0dd802..13561864027316 100644 --- a/src/callback_queue-inl.h +++ b/src/callback_queue-inl.h @@ -10,8 +10,8 @@ namespace node { template template std::unique_ptr::Callback> -CallbackQueue::CreateCallback(Fn&& fn, bool refed) { - return std::make_unique>(std::move(fn), refed); +CallbackQueue::CreateCallback(Fn&& fn, CallbackFlags::Flags flags) { + return std::make_unique>(std::move(fn), flags); } template @@ -57,12 +57,12 @@ size_t CallbackQueue::size() const { } template -CallbackQueue::Callback::Callback(bool refed) - : refed_(refed) {} +CallbackQueue::Callback::Callback(CallbackFlags::Flags flags) + : flags_(flags) {} template -bool CallbackQueue::Callback::is_refed() const { - return refed_; +CallbackFlags::Flags CallbackQueue::Callback::flags() const { + return flags_; } template @@ -80,8 +80,8 @@ void CallbackQueue::Callback::set_next( template template CallbackQueue::CallbackImpl::CallbackImpl( - Fn&& callback, bool refed) - : Callback(refed), + Fn&& callback, CallbackFlags::Flags flags) + : Callback(flags), callback_(std::move(callback)) {} template diff --git a/src/callback_queue.h b/src/callback_queue.h index ebf975e6391d13..e5694d5e1fe56a 100644 --- a/src/callback_queue.h +++ b/src/callback_queue.h @@ -7,6 +7,13 @@ namespace node { +namespace CallbackFlags { +enum Flags { + kUnrefed = 0, + kRefed = 1, +}; +} + // A queue of C++ functions that take Args... as arguments and return R // (this is similar to the signature of std::function). // New entries are added using `CreateCallback()`/`Push()`, and removed using @@ -18,25 +25,26 @@ class CallbackQueue { public: class Callback { public: - explicit inline Callback(bool refed); + explicit inline Callback(CallbackFlags::Flags flags); virtual ~Callback() = default; virtual R Call(Args... args) = 0; - inline bool is_refed() const; + inline CallbackFlags::Flags flags() const; private: inline std::unique_ptr get_next(); inline void set_next(std::unique_ptr next); - bool refed_; + CallbackFlags::Flags flags_; std::unique_ptr next_; friend class CallbackQueue; }; template - inline std::unique_ptr CreateCallback(Fn&& fn, bool refed); + inline std::unique_ptr CreateCallback( + Fn&& fn, CallbackFlags::Flags); inline std::unique_ptr Shift(); inline void Push(std::unique_ptr cb); @@ -51,7 +59,7 @@ class CallbackQueue { template class CallbackImpl final : public Callback { public: - CallbackImpl(Fn&& callback, bool refed); + CallbackImpl(Fn&& callback, CallbackFlags::Flags flags); R Call(Args... args) override; private: diff --git a/src/env-inl.h b/src/env-inl.h index c6ef9dc13ab6f1..21966bea2692b2 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -732,29 +732,21 @@ inline void IsolateData::set_options( } template -void Environment::CreateImmediate(Fn&& cb, bool ref) { - auto callback = native_immediates_.CreateCallback(std::move(cb), ref); +void Environment::SetImmediate(Fn&& cb, CallbackFlags::Flags flags) { + auto callback = native_immediates_.CreateCallback(std::move(cb), flags); native_immediates_.Push(std::move(callback)); -} - -template -void Environment::SetImmediate(Fn&& cb) { - CreateImmediate(std::move(cb), true); - if (immediate_info()->ref_count() == 0) - ToggleImmediateRef(true); - immediate_info()->ref_count_inc(1); -} - -template -void Environment::SetUnrefImmediate(Fn&& cb) { - CreateImmediate(std::move(cb), false); + if (flags & CallbackFlags::kRefed) { + if (immediate_info()->ref_count() == 0) + ToggleImmediateRef(true); + immediate_info()->ref_count_inc(1); + } } template -void Environment::SetImmediateThreadsafe(Fn&& cb, bool refed) { - auto callback = - native_immediates_threadsafe_.CreateCallback(std::move(cb), refed); +void Environment::SetImmediateThreadsafe(Fn&& cb, CallbackFlags::Flags flags) { + auto callback = native_immediates_threadsafe_.CreateCallback( + std::move(cb), flags); { Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_); native_immediates_threadsafe_.Push(std::move(callback)); @@ -764,8 +756,8 @@ void Environment::SetImmediateThreadsafe(Fn&& cb, bool refed) { template void Environment::RequestInterrupt(Fn&& cb) { - auto callback = - native_immediates_interrupts_.CreateCallback(std::move(cb), false); + auto callback = native_immediates_interrupts_.CreateCallback( + std::move(cb), CallbackFlags::kRefed); { Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_); native_immediates_interrupts_.Push(std::move(callback)); diff --git a/src/env.cc b/src/env.cc index 34f5adccc2e0b5..61557f0153d2d1 100644 --- a/src/env.cc +++ b/src/env.cc @@ -543,7 +543,7 @@ void Environment::CleanupHandles() { Isolate::DisallowJavascriptExecutionScope disallow_js(isolate(), Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE); - RunAndClearNativeImmediates(true /* skip SetUnrefImmediate()s */); + RunAndClearNativeImmediates(true /* skip unrefed SetImmediate()s */); for (ReqWrapBase* request : req_wrap_queue_) request->Cancel(); @@ -673,10 +673,11 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) { TryCatchScope try_catch(this); DebugSealHandleScope seal_handle_scope(isolate()); while (auto head = queue->Shift()) { - if (head->is_refed()) + bool is_refed = head->flags() & CallbackFlags::kRefed; + if (is_refed) ref_count++; - if (head->is_refed() || !only_refed) + if (is_refed || !only_refed) head->Call(this); head.reset(); // Destroy now so that this is also observed by try_catch. diff --git a/src/env.h b/src/env.h index 27d99e53e8ea65..a2efa7d21587f9 100644 --- a/src/env.h +++ b/src/env.h @@ -1180,12 +1180,12 @@ class Environment : public MemoryRetainer { // Unlike the JS setImmediate() function, nested SetImmediate() calls will // be run without returning control to the event loop, similar to nextTick(). template - inline void SetImmediate(Fn&& cb); - template - inline void SetUnrefImmediate(Fn&& cb); + inline void SetImmediate( + Fn&& cb, CallbackFlags::Flags flags = CallbackFlags::kRefed); template // This behaves like SetImmediate() but can be called from any thread. - inline void SetImmediateThreadsafe(Fn&& cb, bool refed = true); + inline void SetImmediateThreadsafe( + Fn&& cb, CallbackFlags::Flags flags = CallbackFlags::kRefed); // This behaves like V8's Isolate::RequestInterrupt(), but also accounts for // the event loop (i.e. combines the V8 function with SetImmediate()). // The passed callback may not throw exceptions. @@ -1265,9 +1265,6 @@ class Environment : public MemoryRetainer { std::shared_ptr); private: - template - inline void CreateImmediate(Fn&& cb, bool ref); - inline void ThrowError(v8::Local (*fun)(v8::Local), const char* errmsg); diff --git a/src/node_dir.cc b/src/node_dir.cc index 92d6bc96e48ad5..3719706efd6006 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -126,10 +126,10 @@ inline void DirHandle::GCClose() { // to notify that the file descriptor was gc'd. We want to be noisy about // this because not explicitly closing the DirHandle is a bug. - env()->SetUnrefImmediate([](Environment* env) { + env()->SetImmediate([](Environment* env) { ProcessEmitWarning(env, "Closing directory handle on garbage collection"); - }); + }, CallbackFlags::kUnrefed); } void AfterClose(uv_fs_t* req) { diff --git a/src/node_file.cc b/src/node_file.cc index b632a5590435de..6d97abc2c8605c 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -221,11 +221,12 @@ inline void FileHandle::Close() { // If the close was successful, we still want to emit a process warning // to notify that the file descriptor was gc'd. We want to be noisy about // this because not explicitly closing the FileHandle is a bug. - env()->SetUnrefImmediate([detail](Environment* env) { + + env()->SetImmediate([detail](Environment* env) { ProcessEmitWarning(env, "Closing file descriptor %d on garbage collection", detail.fd); - }); + }, CallbackFlags::kUnrefed); } void FileHandle::CloseReq::Resolve() { diff --git a/src/node_perf.cc b/src/node_perf.cc index 4b8bf2a8a7c913..fb3b2c43acdd9f 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -282,9 +282,9 @@ void MarkGarbageCollectionEnd(Isolate* isolate, static_cast(flags), state->performance_last_gc_start_mark, PERFORMANCE_NOW()); - env->SetUnrefImmediate([entry = std::move(entry)](Environment* env) mutable { + env->SetImmediate([entry = std::move(entry)](Environment* env) mutable { PerformanceGCCallback(env, std::move(entry)); - }); + }, CallbackFlags::kUnrefed); } void GarbageCollectionCleanupHook(void* data) { diff --git a/src/node_worker.cc b/src/node_worker.cc index 7f4aec6a0bebc8..706a63b03d5612 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -764,7 +764,7 @@ void Worker::TakeHeapSnapshot(const FunctionCallbackInfo& args) { env, std::move(snapshot)); Local args[] = { stream->object() }; taker->MakeCallback(env->ondone_string(), arraysize(args), args); - }, /* refed */ false); + }, CallbackFlags::kUnrefed); }); args.GetReturnValue().Set(scheduled ? taker->object() : Local()); } diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index a15e56a772ed19..75aa4b7d840e12 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -232,10 +232,10 @@ TEST_F(EnvironmentTest, SetImmediateCleanup) { EXPECT_EQ(env_arg, *env); called++; }); - (*env)->SetUnrefImmediate([&](node::Environment* env_arg) { + (*env)->SetImmediate([&](node::Environment* env_arg) { EXPECT_EQ(env_arg, *env); called_unref++; - }); + }, node::CallbackFlags::kUnrefed); } EXPECT_EQ(called, 1);