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

src: use uv_async_t for WeakRefs · nodejs/node@9e9e48b · GitHub

/ node Public

Commit 9e9e48b

Browse files
addaleax
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

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
@@ -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()(

‎src/env.cc‎

Lines 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) {

‎src/env.h‎

Lines 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_;

‎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