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 9e9e48b

Browse filesBrowse files
addaleaxaddaleax
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 94e4cbd commit 9e9e48b
Copy full SHA for 9e9e48b

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
@@ -1118,6 +1118,7 @@ void Environment::RemoveCleanupHook(void (*fn)(void*), void* arg) {
11181118
inline void Environment::RegisterFinalizationGroupForCleanup(
11191119
v8::Local<v8::FinalizationGroup> group) {
11201120
cleanup_finalization_groups_.emplace_back(isolate(), group);
1121+
uv_async_send(&cleanup_finalization_groups_async_);
11211122
}
11221123

11231124
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
@@ -460,8 +460,17 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
460460
// will be recorded with state=IDLE.
461461
uv_prepare_init(event_loop(), &idle_prepare_handle_);
462462
uv_check_init(event_loop(), &idle_check_handle_);
463+
uv_async_init(
464+
event_loop(),
465+
&cleanup_finalization_groups_async_,
466+
[](uv_async_t* async) {
467+
Environment* env = ContainerOf(
468+
&Environment::cleanup_finalization_groups_async_, async);
469+
env->CleanupFinalizationGroups();
470+
});
463471
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
464472
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_check_handle_));
473+
uv_unref(reinterpret_cast<uv_handle_t*>(&cleanup_finalization_groups_async_));
465474

466475
thread_stopper()->Install(
467476
this, static_cast<void*>(this), [](uv_async_t* handle) {
@@ -524,6 +533,10 @@ void Environment::RegisterHandleCleanups() {
524533
reinterpret_cast<uv_handle_t*>(&idle_check_handle_),
525534
close_and_finish,
526535
nullptr);
536+
RegisterHandleCleanup(
537+
reinterpret_cast<uv_handle_t*>(&cleanup_finalization_groups_async_),
538+
close_and_finish,
539+
nullptr);
527540
}
528541

529542
void Environment::CleanupHandles() {
@@ -1040,19 +1053,27 @@ char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
10401053
return new_data;
10411054
}
10421055

1043-
bool Environment::RunWeakRefCleanup() {
1056+
void Environment::RunWeakRefCleanup() {
10441057
isolate()->ClearKeptObjects();
1058+
}
10451059

1046-
while (!cleanup_finalization_groups_.empty()) {
1060+
void Environment::CleanupFinalizationGroups() {
1061+
HandleScope handle_scope(isolate());
1062+
Context::Scope context_scope(context());
1063+
TryCatchScope try_catch(this);
1064+
1065+
while (!cleanup_finalization_groups_.empty() && can_call_into_js()) {
10471066
Local<FinalizationGroup> fg =
10481067
cleanup_finalization_groups_.front().Get(isolate());
10491068
cleanup_finalization_groups_.pop_front();
10501069
if (!FinalizationGroup::Cleanup(fg).FromMaybe(false)) {
1051-
return false;
1070+
if (try_catch.HasCaught() && !try_catch.HasTerminated())
1071+
errors::TriggerUncaughtException(isolate(), try_catch);
1072+
// Re-schedule the execution of the remainder of the queue.
1073+
uv_async_send(&cleanup_finalization_groups_async_);
1074+
return;
10521075
}
10531076
}
1054-
1055-
return true;
10561077
}
10571078

10581079
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
@@ -1128,7 +1128,8 @@ class Environment : public MemoryRetainer {
11281128
void RunAtExitCallbacks();
11291129

11301130
void RegisterFinalizationGroupForCleanup(v8::Local<v8::FinalizationGroup> fg);
1131-
bool RunWeakRefCleanup();
1131+
void RunWeakRefCleanup();
1132+
void CleanupFinalizationGroups();
11321133

11331134
// Strings and private symbols are shared across shared contexts
11341135
// The getters simply proxy to the per-isolate primitive.
@@ -1270,6 +1271,7 @@ class Environment : public MemoryRetainer {
12701271
uv_idle_t immediate_idle_handle_;
12711272
uv_prepare_t idle_prepare_handle_;
12721273
uv_check_t idle_check_handle_;
1274+
uv_async_t cleanup_finalization_groups_async_;
12731275
bool profiler_idle_notifier_started_ = false;
12741276

12751277
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.