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 67a9742

Browse filesBrowse files
bnoordhuisMylesBorins
bnoordhuis
authored and
MylesBorins
committed
src: prevent persistent handle resource leaks
Replace v8::Persistent with node::Persistent, a specialization that resets the persistent handle on destruction. Prevents accidental resource leaks when forgetting to call .Reset() manually. I'm fairly confident this commit fixes a number of resource leaks that have gone undiagnosed so far. Backport-PR-URL: #19185 PR-URL: #18656 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 08bcdde commit 67a9742
Copy full SHA for 67a9742
Expand file treeCollapse file tree

25 files changed

+97
-82
lines changed
Open diff view settings
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,9 +359,10 @@
359359
'src/node_internals.h',
360360
'src/node_javascript.h',
361361
'src/node_mutex.h',
362-
'src/node_platform.h',
363362
'src/node_perf.h',
364363
'src/node_perf_common.h',
364+
'src/node_persistent.h',
365+
'src/node_platform.h',
365366
'src/node_root_certs.h',
366367
'src/node_version.h',
367368
'src/node_watchdog.h',
Collapse file

‎src/async_wrap.cc‎

Copy file name to clipboardExpand all lines: src/async_wrap.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,8 @@ static void DisablePromiseHook(const FunctionCallbackInfo<Value>& args) {
410410
class DestroyParam {
411411
public:
412412
double asyncId;
413-
v8::Persistent<Object> target;
414-
v8::Persistent<Object> propBag;
413+
Persistent<Object> target;
414+
Persistent<Object> propBag;
415415
};
416416

417417

Collapse file

‎src/base_object-inl.h‎

Copy file name to clipboardExpand all lines: src/base_object-inl.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ inline BaseObject::~BaseObject() {
4747
}
4848

4949

50-
inline v8::Persistent<v8::Object>& BaseObject::persistent() {
50+
inline Persistent<v8::Object>& BaseObject::persistent() {
5151
return persistent_handle_;
5252
}
5353

Collapse file

‎src/base_object.h‎

Copy file name to clipboardExpand all lines: src/base_object.h
+3-7Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
2626

27+
#include "node_persistent.h"
2728
#include "v8.h"
2829

2930
namespace node {
@@ -39,12 +40,7 @@ class BaseObject {
3940
// persistent.IsEmpty() is true.
4041
inline v8::Local<v8::Object> object();
4142

42-
// The parent class is responsible for calling .Reset() on destruction
43-
// when the persistent handle is strong because there is no way for
44-
// BaseObject to know when the handle goes out of scope.
45-
// Weak handles have been reset by the time the destructor runs but
46-
// calling .Reset() again is harmless.
47-
inline v8::Persistent<v8::Object>& persistent();
43+
inline Persistent<v8::Object>& persistent();
4844

4945
inline Environment* env() const;
5046

@@ -71,7 +67,7 @@ class BaseObject {
7167
// position of members in memory are predictable. For more information please
7268
// refer to `doc/guides/node-postmortem-support.md`
7369
friend int GenDebugSymbols();
74-
v8::Persistent<v8::Object> persistent_handle_;
70+
Persistent<v8::Object> persistent_handle_;
7571
Environment* env_;
7672
};
7773

Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,8 +541,8 @@ void Environment::CreateImmediate(native_immediate_callback cb,
541541
native_immediate_callbacks_.push_back({
542542
cb,
543543
data,
544-
std::unique_ptr<v8::Persistent<v8::Object>>(obj.IsEmpty() ?
545-
nullptr : new v8::Persistent<v8::Object>(isolate_, obj)),
544+
std::unique_ptr<Persistent<v8::Object>>(obj.IsEmpty() ?
545+
nullptr : new Persistent<v8::Object>(isolate_, obj)),
546546
ref
547547
});
548548
immediate_info()->count_inc(1);
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+2-3Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,7 @@ class Environment {
800800
struct NativeImmediateCallback {
801801
native_immediate_callback cb_;
802802
void* data_;
803-
std::unique_ptr<v8::Persistent<v8::Object>> keep_alive_;
803+
std::unique_ptr<Persistent<v8::Object>> keep_alive_;
804804
bool refed_;
805805
};
806806
std::vector<NativeImmediateCallback> native_immediate_callbacks_;
@@ -811,8 +811,7 @@ class Environment {
811811
v8::Local<v8::Promise> promise,
812812
v8::Local<v8::Value> parent);
813813

814-
#define V(PropertyName, TypeName) \
815-
v8::Persistent<TypeName> PropertyName ## _;
814+
#define V(PropertyName, TypeName) Persistent<TypeName> PropertyName ## _;
816815
ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
817816
#undef V
818817

Collapse file

‎src/inspector_agent.cc‎

Copy file name to clipboardExpand all lines: src/inspector_agent.cc
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ using v8::HandleScope;
3030
using v8::Isolate;
3131
using v8::Local;
3232
using v8::Object;
33-
using v8::Persistent;
3433
using v8::String;
3534
using v8::Value;
3635

Collapse file

‎src/inspector_agent.h‎

Copy file name to clipboardExpand all lines: src/inspector_agent.h
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class Agent {
9797

9898
private:
9999
void ToggleAsyncHook(v8::Isolate* isolate,
100-
const v8::Persistent<v8::Function>& fn);
100+
const Persistent<v8::Function>& fn);
101101

102102
node::Environment* parent_env_;
103103
std::unique_ptr<NodeInspectorClient> client_;
@@ -109,8 +109,8 @@ class Agent {
109109

110110
bool pending_enable_async_hook_;
111111
bool pending_disable_async_hook_;
112-
v8::Persistent<v8::Function> enable_async_hook_function_;
113-
v8::Persistent<v8::Function> disable_async_hook_function_;
112+
Persistent<v8::Function> enable_async_hook_function_;
113+
Persistent<v8::Function> disable_async_hook_function_;
114114
};
115115

116116
} // namespace inspector
Collapse file

‎src/inspector_js_api.cc‎

Copy file name to clipboardExpand all lines: src/inspector_js_api.cc
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ using v8::Local;
1919
using v8::MaybeLocal;
2020
using v8::NewStringType;
2121
using v8::Object;
22-
using v8::Persistent;
2322
using v8::String;
2423
using v8::Value;
2524

Collapse file

‎src/module_wrap.h‎

Copy file name to clipboardExpand all lines: src/module_wrap.h
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ class ModuleWrap : public BaseObject {
4949
v8::Local<v8::String> specifier,
5050
v8::Local<v8::Module> referrer);
5151

52-
v8::Persistent<v8::Module> module_;
53-
v8::Persistent<v8::String> url_;
52+
Persistent<v8::Module> module_;
53+
Persistent<v8::String> url_;
5454
bool linked_ = false;
55-
std::unordered_map<std::string, v8::Persistent<v8::Promise>> resolve_cache_;
56-
v8::Persistent<v8::Context> context_;
55+
std::unordered_map<std::string, Persistent<v8::Promise>> resolve_cache_;
56+
Persistent<v8::Context> context_;
5757
};
5858

5959
} // namespace loader

0 commit comments

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