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 e253edb

Browse filesBrowse files
addaleaxaddaleax
committed
src: make CleanupHandles() tear down handles/reqs
Previously, handles would not be closed when the current `Environment` stopped, which is acceptable in a single-`Environment`-per-process situation, but would otherwise create memory and file descriptor leaks. Also, introduce a generic way to close handles via the `Environment::CloseHandle()` function, which automatically keeps track of whether a close callback has been called yet or not. Many thanks for Stephen Belanger for reviewing the original version of this commit in the Ayo.js project. Refs: ayojs/ayo#85 PR-URL: #19377 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent ba26958 commit e253edb
Copy full SHA for e253edb

File tree

Expand file treeCollapse file tree

13 files changed

+90
-45
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

13 files changed

+90
-45
lines changed
Open diff view settings
Collapse file

‎src/cares_wrap.cc‎

Copy file name to clipboardExpand all lines: src/cares_wrap.cc
+6-12Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,8 @@ void ares_poll_cb(uv_poll_t* watcher, int status, int events) {
267267
}
268268

269269

270-
void ares_poll_close_cb(uv_handle_t* watcher) {
271-
node_ares_task* task = ContainerOf(&node_ares_task::poll_watcher,
272-
reinterpret_cast<uv_poll_t*>(watcher));
270+
void ares_poll_close_cb(uv_poll_t* watcher) {
271+
node_ares_task* task = ContainerOf(&node_ares_task::poll_watcher, watcher);
273272
free(task);
274273
}
275274

@@ -347,8 +346,7 @@ void ares_sockstate_cb(void* data,
347346
"When an ares socket is closed we should have a handle for it");
348347

349348
channel->task_list()->erase(it);
350-
uv_close(reinterpret_cast<uv_handle_t*>(&task->poll_watcher),
351-
ares_poll_close_cb);
349+
channel->env()->CloseHandle(&task->poll_watcher, ares_poll_close_cb);
352350

353351
if (channel->task_list()->empty()) {
354352
uv_timer_stop(channel->timer_handle());
@@ -517,10 +515,7 @@ ChannelWrap::~ChannelWrap() {
517515
void ChannelWrap::CleanupTimer() {
518516
if (timer_handle_ == nullptr) return;
519517

520-
uv_close(reinterpret_cast<uv_handle_t*>(timer_handle_),
521-
[](uv_handle_t* handle) {
522-
delete reinterpret_cast<uv_timer_t*>(handle);
523-
});
518+
env()->CloseHandle(timer_handle_, [](uv_timer_t* handle){ delete handle; });
524519
timer_handle_ = nullptr;
525520
}
526521

@@ -610,8 +605,7 @@ class QueryWrap : public AsyncWrap {
610605
static_cast<void*>(this));
611606
}
612607

613-
static void CaresAsyncClose(uv_handle_t* handle) {
614-
uv_async_t* async = reinterpret_cast<uv_async_t*>(handle);
608+
static void CaresAsyncClose(uv_async_t* async) {
615609
auto data = static_cast<struct CaresAsyncData*>(async->data);
616610
delete data->wrap;
617611
delete data;
@@ -636,7 +630,7 @@ class QueryWrap : public AsyncWrap {
636630
free(host);
637631
}
638632

639-
uv_close(reinterpret_cast<uv_handle_t*>(handle), CaresAsyncClose);
633+
wrap->env()->CloseHandle(handle, CaresAsyncClose);
640634
}
641635

642636
static void Callback(void *arg, int status, int timeouts,
Collapse file

‎src/connection_wrap.h‎

Copy file name to clipboardExpand all lines: src/connection_wrap.h
-2Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ class ConnectionWrap : public LibuvStreamWrap {
2323
ConnectionWrap(Environment* env,
2424
v8::Local<v8::Object> object,
2525
ProviderType provider);
26-
~ConnectionWrap() {
27-
}
2826

2927
UVType handle_;
3028
};
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+20-2Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,26 @@ inline void Environment::RegisterHandleCleanup(uv_handle_t* handle,
349349
handle_cleanup_queue_.push_back(HandleCleanup{handle, cb, arg});
350350
}
351351

352-
inline void Environment::FinishHandleCleanup(uv_handle_t* handle) {
353-
handle_cleanup_waiting_--;
352+
template <typename T, typename OnCloseCallback>
353+
inline void Environment::CloseHandle(T* handle, OnCloseCallback callback) {
354+
handle_cleanup_waiting_++;
355+
static_assert(sizeof(T) >= sizeof(uv_handle_t), "T is a libuv handle");
356+
static_assert(offsetof(T, data) == offsetof(uv_handle_t, data),
357+
"T is a libuv handle");
358+
static_assert(offsetof(T, close_cb) == offsetof(uv_handle_t, close_cb),
359+
"T is a libuv handle");
360+
struct CloseData {
361+
Environment* env;
362+
OnCloseCallback callback;
363+
void* original_data;
364+
};
365+
handle->data = new CloseData { this, callback, handle->data };
366+
uv_close(reinterpret_cast<uv_handle_t*>(handle), [](uv_handle_t* handle) {
367+
std::unique_ptr<CloseData> data { static_cast<CloseData*>(handle->data) };
368+
data->env->handle_cleanup_waiting_--;
369+
handle->data = data->original_data;
370+
data->callback(reinterpret_cast<T*>(handle));
371+
});
354372
}
355373

356374
inline uv_loop_t* Environment::event_loop() const {
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+12-8Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,7 @@ void Environment::RegisterHandleCleanups() {
209209
void* arg) {
210210
handle->data = env;
211211

212-
uv_close(handle, [](uv_handle_t* handle) {
213-
static_cast<Environment*>(handle->data)->FinishHandleCleanup(handle);
214-
});
212+
env->CloseHandle(handle, [](uv_handle_t* handle) {});
215213
};
216214

217215
RegisterHandleCleanup(
@@ -233,13 +231,17 @@ void Environment::RegisterHandleCleanups() {
233231
}
234232

235233
void Environment::CleanupHandles() {
236-
for (HandleCleanup& hc : handle_cleanup_queue_) {
237-
handle_cleanup_waiting_++;
234+
for (ReqWrap<uv_req_t>* request : req_wrap_queue_)
235+
request->Cancel();
236+
237+
for (HandleWrap* handle : handle_wrap_queue_)
238+
handle->Close();
239+
240+
for (HandleCleanup& hc : handle_cleanup_queue_)
238241
hc.cb_(this, hc.handle_, hc.arg_);
239-
}
240242
handle_cleanup_queue_.clear();
241243

242-
while (handle_cleanup_waiting_ != 0)
244+
while (handle_cleanup_waiting_ != 0 || !handle_wrap_queue_.IsEmpty())
243245
uv_run(event_loop(), UV_RUN_ONCE);
244246
}
245247

@@ -306,6 +308,8 @@ void Environment::PrintSyncTrace() const {
306308
}
307309

308310
void Environment::RunCleanup() {
311+
CleanupHandles();
312+
309313
while (!cleanup_hooks_.empty()) {
310314
// Copy into a vector, since we can't sort an unordered_set in-place.
311315
std::vector<CleanupHookCallback> callbacks(
@@ -329,8 +333,8 @@ void Environment::RunCleanup() {
329333

330334
cb.fn_(cb.arg_);
331335
cleanup_hooks_.erase(cb);
332-
CleanupHandles();
333336
}
337+
CleanupHandles();
334338
}
335339
}
336340

Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -577,10 +577,14 @@ class Environment {
577577

578578
void RegisterHandleCleanups();
579579
void CleanupHandles();
580+
581+
// Register clean-up cb to be called on environment destruction.
580582
inline void RegisterHandleCleanup(uv_handle_t* handle,
581583
HandleCleanupCb cb,
582584
void *arg);
583-
inline void FinishHandleCleanup(uv_handle_t* handle);
585+
586+
template <typename T, typename OnCloseCallback>
587+
inline void CloseHandle(T* handle, OnCloseCallback callback);
584588

585589
inline void AssignToContext(v8::Local<v8::Context> context,
586590
const ContextInfo& info);
Collapse file

‎src/fs_event_wrap.cc‎

Copy file name to clipboardExpand all lines: src/fs_event_wrap.cc
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,12 @@ FSEventWrap::FSEventWrap(Environment* env, Local<Object> object)
7878
: HandleWrap(env,
7979
object,
8080
reinterpret_cast<uv_handle_t*>(&handle_),
81-
AsyncWrap::PROVIDER_FSEVENTWRAP) {}
81+
AsyncWrap::PROVIDER_FSEVENTWRAP) {
82+
MarkAsUninitialized();
83+
}
8284

8385

8486
FSEventWrap::~FSEventWrap() {
85-
CHECK_EQ(initialized_, false);
8687
}
8788

8889
void FSEventWrap::GetInitialized(const FunctionCallbackInfo<Value>& args) {
@@ -153,6 +154,7 @@ void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) {
153154
}
154155

155156
err = uv_fs_event_start(&wrap->handle_, OnEvent, *path, flags);
157+
wrap->MarkAsInitialized();
156158
wrap->initialized_ = true;
157159

158160
if (err != 0) {
Collapse file

‎src/handle_wrap.cc‎

Copy file name to clipboardExpand all lines: src/handle_wrap.cc
+25-12Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,29 +61,40 @@ void HandleWrap::HasRef(const FunctionCallbackInfo<Value>& args) {
6161

6262

6363
void HandleWrap::Close(const FunctionCallbackInfo<Value>& args) {
64-
Environment* env = Environment::GetCurrent(args);
65-
6664
HandleWrap* wrap;
6765
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
6866

69-
// Guard against uninitialized handle or double close.
70-
if (!IsAlive(wrap))
71-
return;
67+
wrap->Close(args[0]);
68+
}
7269

73-
if (wrap->state_ != kInitialized)
70+
void HandleWrap::Close(v8::Local<v8::Value> close_callback) {
71+
if (state_ != kInitialized)
7472
return;
7573

76-
CHECK_EQ(false, wrap->persistent().IsEmpty());
77-
uv_close(wrap->handle_, OnClose);
78-
wrap->state_ = kClosing;
74+
CHECK_EQ(false, persistent().IsEmpty());
75+
uv_close(handle_, OnClose);
76+
state_ = kClosing;
7977

80-
if (args[0]->IsFunction()) {
81-
wrap->object()->Set(env->onclose_string(), args[0]);
82-
wrap->state_ = kClosingWithCallback;
78+
if (!close_callback.IsEmpty() && close_callback->IsFunction()) {
79+
object()->Set(env()->context(), env()->onclose_string(), close_callback)
80+
.FromJust();
81+
state_ = kClosingWithCallback;
8382
}
8483
}
8584

8685

86+
void HandleWrap::MarkAsInitialized() {
87+
env()->handle_wrap_queue()->PushBack(this);
88+
state_ = kInitialized;
89+
}
90+
91+
92+
void HandleWrap::MarkAsUninitialized() {
93+
handle_wrap_queue_.Remove();
94+
state_ = kClosed;
95+
}
96+
97+
8798
HandleWrap::HandleWrap(Environment* env,
8899
Local<Object> object,
89100
uv_handle_t* handle,
@@ -110,6 +121,8 @@ void HandleWrap::OnClose(uv_handle_t* handle) {
110121
const bool have_close_callback = (wrap->state_ == kClosingWithCallback);
111122
wrap->state_ = kClosed;
112123

124+
wrap->OnClose();
125+
113126
if (have_close_callback)
114127
wrap->MakeCallback(env->onclose_string(), 0, nullptr);
115128

Collapse file

‎src/handle_wrap.h‎

Copy file name to clipboardExpand all lines: src/handle_wrap.h
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,17 @@ class HandleWrap : public AsyncWrap {
7070

7171
inline uv_handle_t* GetHandle() const { return handle_; }
7272

73+
void Close(v8::Local<v8::Value> close_callback = v8::Local<v8::Value>());
74+
7375
protected:
7476
HandleWrap(Environment* env,
7577
v8::Local<v8::Object> object,
7678
uv_handle_t* handle,
7779
AsyncWrap::ProviderType provider);
80+
virtual void OnClose() {}
81+
82+
void MarkAsInitialized();
83+
void MarkAsUninitialized();
7884

7985
private:
8086
friend class Environment;
Collapse file

‎src/node_stat_watcher.cc‎

Copy file name to clipboardExpand all lines: src/node_stat_watcher.cc
+1-6Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,6 @@ void StatWatcher::Initialize(Environment* env, Local<Object> target) {
7575
}
7676

7777

78-
static void Delete(uv_handle_t* handle) {
79-
delete reinterpret_cast<uv_fs_poll_t*>(handle);
80-
}
81-
82-
8378
StatWatcher::StatWatcher(Environment* env, Local<Object> wrap)
8479
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_STATWATCHER),
8580
watcher_(new uv_fs_poll_t) {
@@ -93,7 +88,7 @@ StatWatcher::~StatWatcher() {
9388
if (IsActive()) {
9489
Stop();
9590
}
96-
uv_close(reinterpret_cast<uv_handle_t*>(watcher_), Delete);
91+
env()->CloseHandle(watcher_, [](uv_fs_poll_t* handle) { delete handle; });
9792
}
9893

9994

Collapse file

‎src/process_wrap.cc‎

Copy file name to clipboardExpand all lines: src/process_wrap.cc
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ class ProcessWrap : public HandleWrap {
8888
object,
8989
reinterpret_cast<uv_handle_t*>(&process_),
9090
AsyncWrap::PROVIDER_PROCESSWRAP) {
91+
MarkAsUninitialized();
9192
}
9293

9394
static void ParseStdioOptions(Environment* env,
@@ -256,6 +257,7 @@ class ProcessWrap : public HandleWrap {
256257
}
257258

258259
int err = uv_spawn(env->event_loop(), &wrap->process_, &options);
260+
wrap->MarkAsInitialized();
259261

260262
if (err == 0) {
261263
CHECK_EQ(wrap->process_.data, wrap);

0 commit comments

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