FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: use uv_async_t for WeakRefs · nodejs/node@50b7f84 · GitHub

/ node Public

Commit 50b7f84

Browse files
addaleax
authored and
BethGriggs
committed
src: use uv_async_t for WeakRefs
Schedule a task on the main event loop, similar to what the HTML spec recommends for browsers. Alternative to #30198 PR-URL: #30616 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 24f7335 commit 50b7f84

5 files changed

Lines changed: 59 additions & 6 deletions

File tree

‎src/env-inl.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,6 +1123,7 @@ void Environment::RemoveCleanupHook(void (*fn)(void*), void* arg) {
11231123
inline void Environment::RegisterFinalizationGroupForCleanup(
11241124
v8::Local<v8::FinalizationGroup> group) {
11251125
cleanup_finalization_groups_.emplace_back(isolate(), group);
1126+
uv_async_send(&cleanup_finalization_groups_async_);
11261127
}
11271128

11281129
size_t CleanupHookCallback::Hash::operator()(

‎src/env.cc‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,17 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
457457
// will be recorded with state=IDLE.
458458
uv_prepare_init(event_loop(), &idle_prepare_handle_);
459459
uv_check_init(event_loop(), &idle_check_handle_);
460+
uv_async_init(
461+
event_loop(),
462+
&cleanup_finalization_groups_async_,
463+
[](uv_async_t* async) {
464+
Environment* env = ContainerOf(
465+
&Environment::cleanup_finalization_groups_async_, async);
466+
env->CleanupFinalizationGroups();
467+
});
460468
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
461469
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_check_handle_));
470+
uv_unref(reinterpret_cast<uv_handle_t*>(&cleanup_finalization_groups_async_));
462471

463472
thread_stopper()->Install(
464473
this, static_cast<void*>(this), [](uv_async_t* handle) {
@@ -521,6 +530,10 @@ void Environment::RegisterHandleCleanups() {
521530
reinterpret_cast<uv_handle_t*>(&idle_check_handle_),
522531
close_and_finish,
523532
nullptr);
533+
RegisterHandleCleanup(
534+
reinterpret_cast<uv_handle_t*>(&cleanup_finalization_groups_async_),
535+
close_and_finish,
536+
nullptr);
524537
}
525538

526539
void Environment::CleanupHandles() {
@@ -1052,19 +1065,27 @@ void Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(
10521065
keep_alive_allocators_->insert(allocator);
10531066
}
10541067

1055-
bool Environment::RunWeakRefCleanup() {
1068+
void Environment::RunWeakRefCleanup() {
10561069
isolate()->ClearKeptObjects();
1070+
}
10571071

1058-
while (!cleanup_finalization_groups_.empty()) {
1072+
void Environment::CleanupFinalizationGroups() {
1073+
HandleScope handle_scope(isolate());
1074+
Context::Scope context_scope(context());
1075+
TryCatchScope try_catch(this);
1076+
1077+
while (!cleanup_finalization_groups_.empty() && can_call_into_js()) {
10591078
Local<FinalizationGroup> fg =
10601079
cleanup_finalization_groups_.front().Get(isolate());
10611080
cleanup_finalization_groups_.pop_front();
10621081
if (!FinalizationGroup::Cleanup(fg).FromMaybe(false)) {
1063-
return false;
1082+
if (try_catch.HasCaught() && !try_catch.HasTerminated())
1083+
errors::TriggerUncaughtException(isolate(), try_catch);
1084+
// Re-schedule the execution of the remainder of the queue.
1085+
uv_async_send(&cleanup_finalization_groups_async_);
1086+
return;
10641087
}
10651088
}
1066-
1067-
return true;
10681089
}
10691090

10701091
void AsyncRequest::Install(Environment* env, void* data, uv_async_cb target) {

‎src/env.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,8 @@ class Environment : public MemoryRetainer {
11301130
void RunAtExitCallbacks();
11311131

11321132
void RegisterFinalizationGroupForCleanup(v8::Local<v8::FinalizationGroup> fg);
1133-
bool RunWeakRefCleanup();
1133+
void RunWeakRefCleanup();
1134+
void CleanupFinalizationGroups();
11341135

11351136
// Strings and private symbols are shared across shared contexts
11361137
// The getters simply proxy to the per-isolate primitive.
@@ -1276,6 +1277,7 @@ class Environment : public MemoryRetainer {
12761277
uv_idle_t immediate_idle_handle_;
12771278
uv_prepare_t idle_prepare_handle_;
12781279
uv_check_t idle_check_handle_;
1280+
uv_async_t cleanup_finalization_groups_async_;
12791281
bool profiler_idle_notifier_started_ = false;
12801282

12811283
AsyncHooks async_hooks_;

‎test/parallel/test-finalization-group-error.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ setTimeout(() => {
1818
name: 'Error',
1919
message: 'test',
2020
});
21+
22+
// Give the callbacks scheduled by global.gc() time to run, as the underlying
23+
// uv_async_t is unref’ed.
24+
setTimeout(() => {}, 200);
2125
}, 200);
2226

2327
process.on('uncaughtException', common.mustCall());

‎test/parallel/test-finalization-group-regular-gc.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Flags: --harmony-weak-refs
2+
'use strict';
3+
require('../common');
4+
const assert = require('assert');
5+
6+
// Test that finalization callbacks do not crash when caused through a regular
7+
// GC (not global.gc()).
8+
9+
const start = Date.now();
10+
const g = new globalThis.FinalizationGroup(() => {
11+
const diff = Date.now() - start;
12+
assert(diff < 10000, `${diff} >= 10000`);
13+
});
14+
g.register({}, 42);
15+
16+
setImmediate(() => {
17+
const arr = [];
18+
// Build up enough memory usage to hopefully trigger a platform task but not
19+
// enough to trigger GC as an interrupt.
20+
while (arr.length < 1000000) arr.push([]);
21+
22+
setTimeout(() => {
23+
g; // Keep reference alive.
24+
}, 200000).unref();
25+
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL