Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d718625

Browse filesBrowse files
Gabriel Schulhoftargos
Gabriel Schulhof
authored and
targos
committed
src: unload addons when environment quits
This is an alternative to #23319 which attaches the loaded addons to the environment and closes them when the environment is destroyed. Backport-PR-URL: #25258 PR-URL: #24861 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 95353c7 commit d718625
Copy full SHA for d718625

File tree

Expand file treeCollapse file tree

8 files changed

+163
-109
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+163
-109
lines changed
Open diff view settings
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,16 @@ inline uv_loop_t* Environment::event_loop() const {
396396
return isolate_data()->event_loop();
397397
}
398398

399+
inline void Environment::TryLoadAddon(
400+
const char* filename,
401+
int flags,
402+
std::function<bool(binding::DLib*)> was_loaded) {
403+
loaded_addons_.emplace_back(filename, flags);
404+
if (!was_loaded(&loaded_addons_.back())) {
405+
loaded_addons_.pop_back();
406+
}
407+
}
408+
399409
inline Environment::AsyncHooks* Environment::async_hooks() {
400410
return &async_hooks_;
401411
}
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,11 @@ Environment::~Environment() {
276276

277277
TRACE_EVENT_NESTABLE_ASYNC_END0(
278278
TRACING_CATEGORY_NODE1(environment), "Environment", this);
279+
280+
// Dereference all addons that were loaded into this environment.
281+
for (binding::DLib& addon : loaded_addons_) {
282+
addon.Close();
283+
}
279284
}
280285

281286
void Environment::Start(const std::vector<std::string>& args,
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,20 @@
3030
#endif
3131
#include "handle_wrap.h"
3232
#include "node.h"
33+
#include "node_binding.h"
3334
#include "node_http2_state.h"
3435
#include "node_options.h"
3536
#include "req_wrap.h"
3637
#include "util.h"
3738
#include "uv.h"
3839
#include "v8.h"
3940

40-
#include <list>
4141
#include <stdint.h>
42-
#include <vector>
42+
#include <functional>
43+
#include <list>
4344
#include <unordered_map>
4445
#include <unordered_set>
46+
#include <vector>
4547

4648
struct nghttp2_rcbuf;
4749

@@ -640,6 +642,9 @@ class Environment {
640642
inline v8::Isolate* isolate() const;
641643
inline uv_loop_t* event_loop() const;
642644
inline uint32_t watched_providers() const;
645+
inline void TryLoadAddon(const char* filename,
646+
int flags,
647+
std::function<bool(binding::DLib*)> was_loaded);
643648

644649
static inline Environment* from_timer_handle(uv_timer_t* handle);
645650
inline uv_timer_t* timer_handle();
@@ -926,6 +931,7 @@ class Environment {
926931
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
927932
const char* errmsg);
928933

934+
std::list<binding::DLib> loaded_addons_;
929935
v8::Isolate* const isolate_;
930936
IsolateData* const isolate_data_;
931937
uv_timer_t timer_handle_;
Collapse file

‎src/node_binding.cc‎

Copy file name to clipboardExpand all lines: src/node_binding.cc
+80-102Lines changed: 80 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@
22
#include "node_internals.h"
33
#include "node_native_module.h"
44

5-
#if defined(__POSIX__)
6-
#include <dlfcn.h>
7-
#endif
8-
95
#if HAVE_OPENSSL
106
#define NODE_BUILTIN_OPENSSL_MODULES(V) V(crypto) V(tls_wrap)
117
#else
@@ -126,31 +122,8 @@ extern "C" void node_module_register(void* m) {
126122

127123
namespace binding {
128124

129-
class DLib {
130-
public:
131-
#ifdef __POSIX__
132-
static const int kDefaultFlags = RTLD_LAZY;
133-
#else
134-
static const int kDefaultFlags = 0;
135-
#endif
136-
137-
inline DLib(const char* filename, int flags)
138-
: filename_(filename), flags_(flags), handle_(nullptr) {}
139-
140-
inline bool Open();
141-
inline void Close();
142-
inline void* GetSymbolAddress(const char* name);
143-
144-
const std::string filename_;
145-
const int flags_;
146-
std::string errmsg_;
147-
void* handle_;
148-
#ifndef __POSIX__
149-
uv_lib_t lib_;
150-
#endif
151-
private:
152-
DISALLOW_COPY_AND_ASSIGN(DLib);
153-
};
125+
DLib::DLib(const char* filename, int flags)
126+
: filename_(filename), flags_(flags), handle_(nullptr) {}
154127

155128
#ifdef __POSIX__
156129
bool DLib::Open() {
@@ -247,87 +220,92 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) {
247220
}
248221

249222
node::Utf8Value filename(env->isolate(), args[1]); // Cast
250-
DLib dlib(*filename, flags);
251-
bool is_opened = dlib.Open();
252-
253-
// Objects containing v14 or later modules will have registered themselves
254-
// on the pending list. Activate all of them now. At present, only one
255-
// module per object is supported.
256-
node_module* const mp =
257-
static_cast<node_module*>(uv_key_get(&thread_local_modpending));
258-
uv_key_set(&thread_local_modpending, nullptr);
259-
260-
if (!is_opened) {
261-
Local<String> errmsg = OneByteString(env->isolate(), dlib.errmsg_.c_str());
262-
dlib.Close();
223+
env->TryLoadAddon(*filename, flags, [&](DLib* dlib) {
224+
const bool is_opened = dlib->Open();
225+
226+
// Objects containing v14 or later modules will have registered themselves
227+
// on the pending list. Activate all of them now. At present, only one
228+
// module per object is supported.
229+
node_module* const mp =
230+
static_cast<node_module*>(uv_key_get(&thread_local_modpending));
231+
uv_key_set(&thread_local_modpending, nullptr);
232+
233+
if (!is_opened) {
234+
Local<String> errmsg =
235+
OneByteString(env->isolate(), dlib->errmsg_.c_str());
236+
dlib->Close();
263237
#ifdef _WIN32
264-
// Windows needs to add the filename into the error message
265-
errmsg = String::Concat(
266-
env->isolate(), errmsg, args[1]->ToString(context).ToLocalChecked());
238+
// Windows needs to add the filename into the error message
239+
errmsg = String::Concat(
240+
env->isolate(), errmsg, args[1]->ToString(context).ToLocalChecked());
267241
#endif // _WIN32
268-
env->isolate()->ThrowException(Exception::Error(errmsg));
269-
return;
270-
}
242+
env->isolate()->ThrowException(Exception::Error(errmsg));
243+
return false;
244+
}
271245

272-
if (mp == nullptr) {
273-
if (auto callback = GetInitializerCallback(&dlib)) {
274-
callback(exports, module, context);
275-
} else if (auto napi_callback = GetNapiInitializerCallback(&dlib)) {
276-
napi_module_register_by_symbol(exports, module, context, napi_callback);
277-
} else {
278-
dlib.Close();
279-
env->ThrowError("Module did not self-register.");
246+
if (mp == nullptr) {
247+
if (auto callback = GetInitializerCallback(dlib)) {
248+
callback(exports, module, context);
249+
} else if (auto napi_callback = GetNapiInitializerCallback(dlib)) {
250+
napi_module_register_by_symbol(exports, module, context, napi_callback);
251+
} else {
252+
dlib->Close();
253+
env->ThrowError("Module did not self-register.");
254+
return false;
255+
}
256+
return true;
280257
}
281-
return;
282-
}
283258

284-
// -1 is used for N-API modules
285-
if ((mp->nm_version != -1) && (mp->nm_version != NODE_MODULE_VERSION)) {
286-
// Even if the module did self-register, it may have done so with the wrong
287-
// version. We must only give up after having checked to see if it has an
288-
// appropriate initializer callback.
289-
if (auto callback = GetInitializerCallback(&dlib)) {
290-
callback(exports, module, context);
291-
return;
259+
// -1 is used for N-API modules
260+
if ((mp->nm_version != -1) && (mp->nm_version != NODE_MODULE_VERSION)) {
261+
// Even if the module did self-register, it may have done so with the
262+
// wrong version. We must only give up after having checked to see if it
263+
// has an appropriate initializer callback.
264+
if (auto callback = GetInitializerCallback(dlib)) {
265+
callback(exports, module, context);
266+
return true;
267+
}
268+
char errmsg[1024];
269+
snprintf(errmsg,
270+
sizeof(errmsg),
271+
"The module '%s'"
272+
"\nwas compiled against a different Node.js version using"
273+
"\nNODE_MODULE_VERSION %d. This version of Node.js requires"
274+
"\nNODE_MODULE_VERSION %d. Please try re-compiling or "
275+
"re-installing\nthe module (for instance, using `npm rebuild` "
276+
"or `npm install`).",
277+
*filename,
278+
mp->nm_version,
279+
NODE_MODULE_VERSION);
280+
281+
// NOTE: `mp` is allocated inside of the shared library's memory, calling
282+
// `dlclose` will deallocate it
283+
dlib->Close();
284+
env->ThrowError(errmsg);
285+
return false;
286+
}
287+
if (mp->nm_flags & NM_F_BUILTIN) {
288+
dlib->Close();
289+
env->ThrowError("Built-in module self-registered.");
290+
return false;
292291
}
293-
char errmsg[1024];
294-
snprintf(errmsg,
295-
sizeof(errmsg),
296-
"The module '%s'"
297-
"\nwas compiled against a different Node.js version using"
298-
"\nNODE_MODULE_VERSION %d. This version of Node.js requires"
299-
"\nNODE_MODULE_VERSION %d. Please try re-compiling or "
300-
"re-installing\nthe module (for instance, using `npm rebuild` "
301-
"or `npm install`).",
302-
*filename,
303-
mp->nm_version,
304-
NODE_MODULE_VERSION);
305-
306-
// NOTE: `mp` is allocated inside of the shared library's memory, calling
307-
// `dlclose` will deallocate it
308-
dlib.Close();
309-
env->ThrowError(errmsg);
310-
return;
311-
}
312-
if (mp->nm_flags & NM_F_BUILTIN) {
313-
dlib.Close();
314-
env->ThrowError("Built-in module self-registered.");
315-
return;
316-
}
317292

318-
mp->nm_dso_handle = dlib.handle_;
319-
mp->nm_link = modlist_addon;
320-
modlist_addon = mp;
293+
mp->nm_dso_handle = dlib->handle_;
294+
mp->nm_link = modlist_addon;
295+
modlist_addon = mp;
321296

322-
if (mp->nm_context_register_func != nullptr) {
323-
mp->nm_context_register_func(exports, module, context, mp->nm_priv);
324-
} else if (mp->nm_register_func != nullptr) {
325-
mp->nm_register_func(exports, module, mp->nm_priv);
326-
} else {
327-
dlib.Close();
328-
env->ThrowError("Module has no declared entry point.");
329-
return;
330-
}
297+
if (mp->nm_context_register_func != nullptr) {
298+
mp->nm_context_register_func(exports, module, context, mp->nm_priv);
299+
} else if (mp->nm_register_func != nullptr) {
300+
mp->nm_register_func(exports, module, mp->nm_priv);
301+
} else {
302+
dlib->Close();
303+
env->ThrowError("Module has no declared entry point.");
304+
return false;
305+
}
306+
307+
return true;
308+
});
331309

332310
// Tell coverity that 'handle' should not be freed when we return.
333311
// coverity[leaked_storage]
Collapse file

‎src/node_binding.h‎

Copy file name to clipboardExpand all lines: src/node_binding.h
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,14 @@
33

44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

6+
#if defined(__POSIX__)
7+
#include <dlfcn.h>
8+
#endif
9+
610
#include "node.h"
11+
#define NAPI_EXPERIMENTAL
712
#include "node_api.h"
13+
#include "util.h"
814
#include "uv.h"
915
#include "v8.h"
1016

@@ -46,6 +52,32 @@ extern bool node_is_initialized;
4652

4753
namespace binding {
4854

55+
class DLib {
56+
public:
57+
#ifdef __POSIX__
58+
static const int kDefaultFlags = RTLD_LAZY;
59+
#else
60+
static const int kDefaultFlags = 0;
61+
#endif
62+
63+
DLib(const char* filename, int flags);
64+
65+
bool Open();
66+
void Close();
67+
void* GetSymbolAddress(const char* name);
68+
69+
const std::string filename_;
70+
const int flags_;
71+
std::string errmsg_;
72+
void* handle_;
73+
#ifndef __POSIX__
74+
uv_lib_t lib_;
75+
#endif
76+
77+
private:
78+
DISALLOW_COPY_AND_ASSIGN(DLib);
79+
};
80+
4981
// Call _register<module_name> functions for all of
5082
// the built-in modules. Because built-in modules don't
5183
// use the __attribute__((constructor)). Need to
@@ -60,5 +92,7 @@ void DLOpen(const v8::FunctionCallbackInfo<v8::Value>& args);
6092

6193
} // namespace node
6294

95+
#include "node_binding.h"
96+
6397
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
6498
#endif // SRC_NODE_BINDING_H_
Collapse file

‎test/abort/test-addon-uv-handle-leak.js‎

Copy file name to clipboardExpand all lines: test/abort/test-addon-uv-handle-leak.js
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ if (!fs.existsSync(bindingPath))
1818
common.skip('binding not built yet');
1919

2020
if (process.argv[2] === 'child') {
21+
22+
// The worker thread loads and then unloads `bindingPath`. Because of this the
23+
// symbols in `bindingPath` are lost when the worker thread quits, but the
24+
// number of open handles in the worker thread's event loop is assessed in the
25+
// main thread afterwards, and the names of the callbacks associated with the
26+
// open handles is retrieved at that time as well. Thus, we require
27+
// `bindingPath` here so that the symbols and their names survive the life
28+
// cycle of the worker thread.
29+
require(bindingPath);
30+
2131
new Worker(`
2232
const binding = require(${JSON.stringify(bindingPath)});
2333
Collapse file

‎test/addons/at-exit/binding.cc‎

Copy file name to clipboardExpand all lines: test/addons/at-exit/binding.cc
+14-2Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ using v8::Isolate;
99
using v8::Local;
1010
using v8::Object;
1111

12+
#if defined(_MSC_VER)
13+
#pragma section(".CRT$XPU", read)
14+
#define NODE_C_DTOR(fn) \
15+
NODE_CTOR_PREFIX void __cdecl fn(void); \
16+
__declspec(dllexport, allocate(".CRT$XPU")) \
17+
void (__cdecl*fn ## _)(void) = fn; \
18+
NODE_CTOR_PREFIX void __cdecl fn(void)
19+
#else
20+
#define NODE_C_DTOR(fn) \
21+
NODE_CTOR_PREFIX void fn(void) __attribute__((destructor)); \
22+
NODE_CTOR_PREFIX void fn(void)
23+
#endif
24+
1225
static char cookie[] = "yum yum";
1326
static int at_exit_cb1_called = 0;
1427
static int at_exit_cb2_called = 0;
@@ -27,7 +40,7 @@ static void at_exit_cb2(void* arg) {
2740
at_exit_cb2_called++;
2841
}
2942

30-
static void sanity_check(void) {
43+
NODE_C_DTOR(sanity_check) {
3144
assert(at_exit_cb1_called == 1);
3245
assert(at_exit_cb2_called == 2);
3346
}
@@ -36,7 +49,6 @@ void init(Local<Object> exports) {
3649
AtExit(at_exit_cb1, exports->GetIsolate());
3750
AtExit(at_exit_cb2, cookie);
3851
AtExit(at_exit_cb2, cookie);
39-
atexit(sanity_check);
4052
}
4153

4254
NODE_MODULE(NODE_GYP_MODULE_NAME, init)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.