Skip to content

Commit

Permalink
src: fix multiple AddLinkedBinding() calls
Browse files Browse the repository at this point in the history
Singly-linked lists are extended at their tail, not their head.
This fixes using more than 2 linked addons at a time.

PR-URL: #39012
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
  • Loading branch information
addaleax authored and targos committed Jun 14, 2021
1 parent eb8f7ee commit 163fcec
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -671,10 +671,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) {
Expand Down
5 changes: 5 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,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_;
}
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,7 @@ class Environment : public MemoryRetainer {
inline void set_stopping(bool value);
inline std::list<node_module>* 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;
Expand Down
32 changes: 32 additions & 0 deletions test/cctest/test_linked_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<v8::Context> 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<v8::Script> script = v8::Script::Compile(
context,
v8::String::NewFromOneByte(isolate_,
reinterpret_cast<const uint8_t*>(run_script))
.ToLocalChecked())
.ToLocalChecked();
v8::Local<v8::Value> 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);
}

0 comments on commit 163fcec

Please sign in to comment.