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 ef9dd60

Browse filesBrowse files
addaleaxtargos
addaleax
authored and
targos
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 c5f22cb commit ef9dd60
Copy full SHA for ef9dd60

File tree

Expand file treeCollapse file tree

5 files changed

+59
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+59
-6
lines changed
Open diff view settings
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+1Lines 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()(
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+26-5Lines 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) {
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+3-1Lines 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_;
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-finalization-group-error.js
+4Lines 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());
Collapse file

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

Copy file name to clipboard
+25Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.