From fdef48849839b7414331e2e03ede788e1958822f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 11 Jun 2021 23:43:48 +0200 Subject: [PATCH 1/2] src: fix multiple AddLinkedBinding() calls Singly-linked lists are extended at their tail, not their head. This fixes using more than 2 linked addons at a time. --- src/api/environment.cc | 6 +++--- src/env-inl.h | 5 +++++ src/env.h | 1 + test/cctest/test_linked_binding.cc | 32 ++++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index baf3065471244a..bd9de560d08120 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -659,10 +659,10 @@ void AddLinkedBinding(Environment* env, const node_module& mod) { CHECK_NOT_NULL(env); Mutex::ScopedLock lock(env->extra_linked_bindings_mutex()); - node_module* prev_head = env->extra_linked_bindings_head(); + node_module* prev_tail = env->extra_linked_bindings_tail(); env->extra_linked_bindings()->push_back(mod); - if (prev_head != nullptr) - prev_head->nm_link = &env->extra_linked_bindings()->back(); + if (prev_tail != nullptr) + prev_tail->nm_link = &env->extra_linked_bindings()->back(); } void AddLinkedBinding(Environment* env, const napi_module& mod) { diff --git a/src/env-inl.h b/src/env-inl.h index e0345b0daf22e4..6fb8f137c37569 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -930,6 +930,11 @@ inline node_module* Environment::extra_linked_bindings_head() { &extra_linked_bindings_.front() : nullptr; } +inline node_module* Environment::extra_linked_bindings_tail() { + return extra_linked_bindings_.size() > 0 ? + &extra_linked_bindings_.back() : nullptr; +} + inline const Mutex& Environment::extra_linked_bindings_mutex() const { return extra_linked_bindings_mutex_; } diff --git a/src/env.h b/src/env.h index a3944eab90a3d5..1dae1f710f8377 100644 --- a/src/env.h +++ b/src/env.h @@ -1208,6 +1208,7 @@ class Environment : public MemoryRetainer { inline void set_stopping(bool value); inline std::list* extra_linked_bindings(); inline node_module* extra_linked_bindings_head(); + inline node_module* extra_linked_bindings_tail(); inline const Mutex& extra_linked_bindings_mutex() const; inline bool filehandle_close_warning() const; diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index 17c020429f0993..536b2ad2b12ac9 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -190,3 +190,35 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { CHECK_EQ(*instance_data, 1); delete instance_data; } + +TEST_F(LinkedBindingTest, ManyBindingsTest) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env test_env {handle_scope, argv}; + + int calls = 0; + AddLinkedBinding(*test_env, "local_linked1", InitializeLocalBinding, &calls); + AddLinkedBinding(*test_env, "local_linked2", InitializeLocalBinding, &calls); + AddLinkedBinding(*test_env, "local_linked3", InitializeLocalBinding, &calls); + AddLinkedBinding(*test_env, local_linked_napi); // Add a N-API addon as well + AddLinkedBinding(*test_env, "local_linked4", InitializeLocalBinding, &calls); + AddLinkedBinding(*test_env, "local_linked5", InitializeLocalBinding, &calls); + + v8::Local context = isolate_->GetCurrentContext(); + + const char* run_script = + "for (let i = 1; i <= 5; i++)process._linkedBinding(`local_linked${i}`);" + "process._linkedBinding('local_linked_napi').hello"; + v8::Local script = v8::Script::Compile( + context, + v8::String::NewFromOneByte(isolate_, + reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); + v8::Local completion_value = script->Run(context).ToLocalChecked(); + v8::String::Utf8Value utf8val(isolate_, completion_value); + CHECK_NOT_NULL(*utf8val); + CHECK_EQ(strcmp(*utf8val, "world"), 0); + CHECK_EQ(calls, 5); +} + From a689e728113d5eecb27c4429d0556422a683bc85 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 11 Jun 2021 23:53:53 +0200 Subject: [PATCH 2/2] fixup! src: fix multiple AddLinkedBinding() calls --- test/cctest/test_linked_binding.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index 536b2ad2b12ac9..7e40068b5db799 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -200,7 +200,7 @@ TEST_F(LinkedBindingTest, ManyBindingsTest) { AddLinkedBinding(*test_env, "local_linked1", InitializeLocalBinding, &calls); AddLinkedBinding(*test_env, "local_linked2", InitializeLocalBinding, &calls); AddLinkedBinding(*test_env, "local_linked3", InitializeLocalBinding, &calls); - AddLinkedBinding(*test_env, local_linked_napi); // Add a N-API addon as well + AddLinkedBinding(*test_env, local_linked_napi); // Add a N-API addon as well. AddLinkedBinding(*test_env, "local_linked4", InitializeLocalBinding, &calls); AddLinkedBinding(*test_env, "local_linked5", InitializeLocalBinding, &calls);