Skip to content

src: add addon entry point symbol cache #20759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

src: add addon entry point symbol cache #20759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 54 additions & 6 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ static std::string trace_enabled_categories; // NOLINT(runtime/string)
static std::string trace_file_pattern = // NOLINT(runtime/string)
"node_trace.${rotation}.log";
static bool abort_on_uncaught_exception = false;
static std::set<void*> addon_entry_points;

// Bit flag used to track security reverts (see node_revert.h)
unsigned int reverted = 0;
Expand Down Expand Up @@ -1258,6 +1259,22 @@ inline napi_addon_register_func GetNapiInitializerCallback(DLib* dlib) {
reinterpret_cast<napi_addon_register_func>(dlib->GetSymbolAddress(name));
}

// Initialize the addon, and close the DLib if this is a second-or-later
// initialization call.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn’t it be the reverse situation? Why do we initialize the addon and then close it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not close the addon. We close the dlopen() handle, because, if the symbol was already in the cache, we know that we've obtainted a dlopen() handle to it in the past, and thus we can conclude that the dlopen() handle we received this time is not a new handle, the closing of which would cause the addon to be unmapped from our address space, but rather a refcounted version of the old handle we received in the past. Thus, closing the new handle is safe.

template <typename Initializer, typename EntryPoint>
inline void CallAddonInitializer(Local<Object> exports,
Local<Value> module,
Local<Context> context,
const Initializer& init,
EntryPoint entry_point,
DLib* dlib) {
init(exports, module, context);
void* generic_entry_point = reinterpret_cast<void*>(entry_point);
if (!(addon_entry_points.insert(generic_entry_point).second)) {
dlib->Close();
}
}

// DLOpen is process.dlopen(module, filename, flags).
// Used to load 'module.node' dynamically shared objects.
//
Expand Down Expand Up @@ -1313,9 +1330,16 @@ static void DLOpen(const FunctionCallbackInfo<Value>& args) {

if (mp == nullptr) {
if (auto callback = GetInitializerCallback(&dlib)) {
callback(exports, module, context);
CallAddonInitializer(exports, module, context, callback, callback, &dlib);
} else if (auto napi_callback = GetNapiInitializerCallback(&dlib)) {
napi_module_register_by_symbol(exports, module, context, napi_callback);
CallAddonInitializer(exports, module, context,
[napi_callback] (Local<Object> exports,
Local<Value> module,
Local<Context> context) {
napi_module_register_by_symbol(exports, module, context,
napi_callback);
},
napi_callback, &dlib);
} else {
dlib.Close();
env->ThrowError("Module did not self-register.");
Expand All @@ -1329,7 +1353,7 @@ static void DLOpen(const FunctionCallbackInfo<Value>& args) {
// version. We must only give up after having checked to see if it has an
// appropriate initializer callback.
if (auto callback = GetInitializerCallback(&dlib)) {
callback(exports, module, context);
CallAddonInitializer(exports, module, context, callback, callback, &dlib);
return;
}
char errmsg[1024];
Expand Down Expand Up @@ -1359,10 +1383,34 @@ static void DLOpen(const FunctionCallbackInfo<Value>& args) {
mp->nm_link = modlist_addon;
modlist_addon = mp;

if (mp->nm_context_register_func != nullptr) {
mp->nm_context_register_func(exports, module, context, mp->nm_priv);
// N-API addons all hide behind a single callback, so its address is not
// suitable for caching. Grab the actual init callback address instead.
if (mp->nm_version == -1) {
auto napi_callback = napi_module_get_entry_point(mp);
CallAddonInitializer(exports, module, context,
[napi_callback] (Local<Object> exports,
Local<Value> module,
Local<Context> context) {
napi_module_register_by_symbol(exports, module, context,
napi_callback);
},
napi_callback, &dlib);
} else if (mp->nm_context_register_func != nullptr) {
CallAddonInitializer(exports, module, context,
[mp] (Local<Object> exports,
Local<Value> module,
Local<Context> context) {
mp->nm_context_register_func(exports, module, context, mp->nm_priv);
},
mp->nm_context_register_func, &dlib);
} else if (mp->nm_register_func != nullptr) {
mp->nm_register_func(exports, module, mp->nm_priv);
CallAddonInitializer(exports, module, context,
[mp] (Local<Object> exports,
Local<Value> module,
Local<Context> context) {
mp->nm_register_func(exports, module, mp->nm_priv);
},
mp->nm_register_func, &dlib);
} else {
dlib.Close();
env->ThrowError("Module has no declared entry point.");
Expand Down
4 changes: 4 additions & 0 deletions src/node_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,10 @@ void napi_module_register_cb(v8::Local<v8::Object> exports,

} // end of anonymous namespace

napi_addon_register_func napi_module_get_entry_point(node::node_module* mp) {
return static_cast<napi_module*>(mp->nm_priv)->nm_register_func;
}

void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
v8::Local<v8::Value> module,
v8::Local<v8::Context> context,
Expand Down
2 changes: 2 additions & 0 deletions src/node_internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,8 @@ void GetGroups(const v8::FunctionCallbackInfo<v8::Value>& args);

} // namespace node

napi_addon_register_func napi_module_get_entry_point(node::node_module* mp);

void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
v8::Local<v8::Value> module,
v8::Local<v8::Context> context,
Expand Down