Skip to content

Commit 1f02617

Browse files
addaleaxBethGriggs
authored andcommitted
deps: V8: cherry-pick 55a01ec7519a
Original commit message: Reland "[weakrefs] Schedule FinalizationGroup cleanup tasks from within V8" Deprecate the following explicit FinalizationGroup APIs in favor of automatic handling of FinalizationGroup cleanup callbacks: - v8::Isolate::SetHostCleanupFinalizationGroupCallback - v8::FinaliationGroup::Cleanup If no HostCleanupFinalizationGroupCallback is set, then FinalizationGroup cleanup callbacks are automatically scheduled by V8 itself as non-nestable foreground tasks. When a Context being disposed, all FinalizationGroups that are associated with it are removed from the dirty list, cancelling scheduled cleanup. This is a reland of 31d8ff7ac5f4a91099f2f06f01e43e9e7aa79bc4 Bug: v8:8179, v8:10190 Change-Id: I704ecf48aeebac1dc2c05ea1c052f6a2560ae332 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2045723 Commit-Queue: Shu-yu Guo <syg@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/heads/master@{#66208} Refs: v8/v8@55a01ec PR-URL: #32885 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent da728c4 commit 1f02617

21 files changed

+328
-76
lines changed

common.gypi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
# Reset this number to 0 on major V8 upgrades.
3737
# Increment by one for each non-official patch applied to deps/v8.
38-
'v8_embedder_string': '-node.21',
38+
'v8_embedder_string': '-node.22',
3939

4040
##### V8 defaults for Node.js #####
4141

deps/v8/BUILD.gn

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2298,6 +2298,8 @@ v8_source_set("v8_base_without_compiler") {
22982298
"src/heap/factory-inl.h",
22992299
"src/heap/factory.cc",
23002300
"src/heap/factory.h",
2301+
"src/heap/finalization-group-cleanup-task.cc",
2302+
"src/heap/finalization-group-cleanup-task.h",
23012303
"src/heap/gc-idle-time-handler.cc",
23022304
"src/heap/gc-idle-time-handler.h",
23032305
"src/heap/gc-tracer.cc",

deps/v8/include/v8.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5885,6 +5885,9 @@ class V8_EXPORT FinalizationGroup : public Object {
58855885
* occurred. Otherwise the result is |true| if the cleanup callback
58865886
* was called successfully. The result is never |false|.
58875887
*/
5888+
V8_DEPRECATED(
5889+
"FinalizationGroup cleanup is automatic if "
5890+
"HostCleanupFinalizationGroupCallback is not set")
58885891
static V8_WARN_UNUSED_RESULT Maybe<bool> Cleanup(
58895892
Local<FinalizationGroup> finalization_group);
58905893
};
@@ -8444,6 +8447,9 @@ class V8_EXPORT Isolate {
84448447
* are ready to be cleaned up and require FinalizationGroup::Cleanup()
84458448
* to be called in a future task.
84468449
*/
8450+
V8_DEPRECATED(
8451+
"FinalizationGroup cleanup is automatic if "
8452+
"HostCleanupFinalizationGroupCallback is not set")
84478453
void SetHostCleanupFinalizationGroupCallback(
84488454
HostCleanupFinalizationGroupCallback callback);
84498455

@@ -9056,10 +9062,10 @@ class V8_EXPORT Isolate {
90569062
void LowMemoryNotification();
90579063

90589064
/**
9059-
* Optional notification that a context has been disposed. V8 uses
9060-
* these notifications to guide the GC heuristic. Returns the number
9061-
* of context disposals - including this one - since the last time
9062-
* V8 had a chance to clean up.
9065+
* Optional notification that a context has been disposed. V8 uses these
9066+
* notifications to guide the GC heuristic and cancel FinalizationGroup
9067+
* cleanup tasks. Returns the number of context disposals - including this one
9068+
* - since the last time V8 had a chance to clean up.
90639069
*
90649070
* The optional parameter |dependant_context| specifies whether the disposed
90659071
* context was depending on state from other contexts or not.

deps/v8/src/api/api.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10950,6 +10950,26 @@ void InvokeFunctionCallback(const v8::FunctionCallbackInfo<v8::Value>& info,
1095010950
callback(info);
1095110951
}
1095210952

10953+
void InvokeFinalizationGroupCleanupFromTask(
10954+
Handle<Context> context, Handle<JSFinalizationGroup> finalization_group,
10955+
Handle<Object> callback) {
10956+
Isolate* isolate = finalization_group->native_context().GetIsolate();
10957+
RuntimeCallTimerScope timer(
10958+
isolate, RuntimeCallCounterId::kFinalizationGroupCleanupFromTask);
10959+
// Do not use ENTER_V8 because this is always called from a running
10960+
// FinalizationGroupCleanupTask within V8 and we should not log it as an API
10961+
// call. This method is implemented here to avoid duplication of the exception
10962+
// handling and microtask running logic in CallDepthScope.
10963+
if (IsExecutionTerminatingCheck(isolate)) return;
10964+
Local<v8::Context> api_context = Utils::ToLocal(context);
10965+
CallDepthScope<true> call_depth_scope(isolate, api_context);
10966+
VMState<OTHER> state(isolate);
10967+
if (JSFinalizationGroup::Cleanup(isolate, finalization_group, callback)
10968+
.IsNothing()) {
10969+
call_depth_scope.Escape();
10970+
}
10971+
}
10972+
1095310973
// Undefine macros for jumbo build.
1095410974
#undef LOG_API
1095510975
#undef ENTER_V8_DO_NOT_USE

deps/v8/src/api/api.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ namespace v8 {
2626

2727
namespace internal {
2828
class JSArrayBufferView;
29+
class JSFinalizationGroup;
2930
} // namespace internal
3031

3132
namespace debug {
@@ -561,6 +562,10 @@ void InvokeAccessorGetterCallback(
561562
void InvokeFunctionCallback(const v8::FunctionCallbackInfo<v8::Value>& info,
562563
v8::FunctionCallback callback);
563564

565+
void InvokeFinalizationGroupCleanupFromTask(
566+
Handle<Context> context, Handle<JSFinalizationGroup> finalization_group,
567+
Handle<Object> callback);
568+
564569
} // namespace internal
565570
} // namespace v8
566571

deps/v8/src/builtins/builtins-weak-refs.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,8 @@ BUILTIN(FinalizationGroupCleanupSome) {
148148
callback = callback_obj;
149149
}
150150

151-
// Don't do set_scheduled_for_cleanup(false); we still have the microtask
152-
// scheduled and don't want to schedule another one in case the user never
153-
// executes microtasks.
151+
// Don't do set_scheduled_for_cleanup(false); we still have the task
152+
// scheduled.
154153
if (JSFinalizationGroup::Cleanup(isolate, finalization_group, callback)
155154
.IsNothing()) {
156155
DCHECK(isolate->has_pending_exception());

deps/v8/src/d8/d8.cc

Lines changed: 5 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -916,16 +916,6 @@ MaybeLocal<Promise> Shell::HostImportModuleDynamically(
916916
return MaybeLocal<Promise>();
917917
}
918918

919-
void Shell::HostCleanupFinalizationGroup(Local<Context> context,
920-
Local<FinalizationGroup> fg) {
921-
Isolate* isolate = context->GetIsolate();
922-
PerIsolateData::Get(isolate)->HostCleanupFinalizationGroup(fg);
923-
}
924-
925-
void PerIsolateData::HostCleanupFinalizationGroup(Local<FinalizationGroup> fg) {
926-
cleanup_finalization_groups_.emplace(isolate_, fg);
927-
}
928-
929919
void Shell::HostInitializeImportMetaObject(Local<Context> context,
930920
Local<Module> module,
931921
Local<Object> meta) {
@@ -1123,15 +1113,6 @@ MaybeLocal<Context> PerIsolateData::GetTimeoutContext() {
11231113
return result;
11241114
}
11251115

1126-
MaybeLocal<FinalizationGroup> PerIsolateData::GetCleanupFinalizationGroup() {
1127-
if (cleanup_finalization_groups_.empty())
1128-
return MaybeLocal<FinalizationGroup>();
1129-
Local<FinalizationGroup> result =
1130-
cleanup_finalization_groups_.front().Get(isolate_);
1131-
cleanup_finalization_groups_.pop();
1132-
return result;
1133-
}
1134-
11351116
PerIsolateData::RealmScope::RealmScope(PerIsolateData* data) : data_(data) {
11361117
data_->realm_count_ = 1;
11371118
data_->realm_current_ = 0;
@@ -1281,8 +1262,11 @@ void Shell::DisposeRealm(const v8::FunctionCallbackInfo<v8::Value>& args,
12811262
int index) {
12821263
Isolate* isolate = args.GetIsolate();
12831264
PerIsolateData* data = PerIsolateData::Get(isolate);
1284-
DisposeModuleEmbedderData(data->realms_[index].Get(isolate));
1265+
Local<Context> context = data->realms_[index].Get(isolate);
1266+
DisposeModuleEmbedderData(context);
12851267
data->realms_[index].Reset();
1268+
// ContextDisposedNotification expects the disposed context to be entered.
1269+
v8::Context::Scope scope(context);
12861270
isolate->ContextDisposedNotification();
12871271
isolate->IdleNotificationDeadline(g_platform->MonotonicallyIncreasingTime());
12881272
}
@@ -2742,8 +2726,6 @@ void SourceGroup::ExecuteInThread() {
27422726
Isolate::CreateParams create_params;
27432727
create_params.array_buffer_allocator = Shell::array_buffer_allocator;
27442728
Isolate* isolate = Isolate::New(create_params);
2745-
isolate->SetHostCleanupFinalizationGroupCallback(
2746-
Shell::HostCleanupFinalizationGroup);
27472729
isolate->SetHostImportModuleDynamicallyCallback(
27482730
Shell::HostImportModuleDynamically);
27492731
isolate->SetHostInitializeImportMetaObjectCallback(
@@ -2889,8 +2871,6 @@ void Worker::ExecuteInThread() {
28892871
Isolate::CreateParams create_params;
28902872
create_params.array_buffer_allocator = Shell::array_buffer_allocator;
28912873
Isolate* isolate = Isolate::New(create_params);
2892-
isolate->SetHostCleanupFinalizationGroupCallback(
2893-
Shell::HostCleanupFinalizationGroup);
28942874
isolate->SetHostImportModuleDynamicallyCallback(
28952875
Shell::HostImportModuleDynamically);
28962876
isolate->SetHostInitializeImportMetaObjectCallback(
@@ -3272,21 +3252,6 @@ bool RunSetTimeoutCallback(Isolate* isolate, bool* did_run) {
32723252
return true;
32733253
}
32743254

3275-
bool RunCleanupFinalizationGroupCallback(Isolate* isolate, bool* did_run) {
3276-
PerIsolateData* data = PerIsolateData::Get(isolate);
3277-
HandleScope handle_scope(isolate);
3278-
while (true) {
3279-
Local<FinalizationGroup> fg;
3280-
if (!data->GetCleanupFinalizationGroup().ToLocal(&fg)) return true;
3281-
*did_run = true;
3282-
TryCatch try_catch(isolate);
3283-
try_catch.SetVerbose(true);
3284-
if (FinalizationGroup::Cleanup(fg).IsNothing()) {
3285-
return false;
3286-
}
3287-
}
3288-
}
3289-
32903255
bool ProcessMessages(
32913256
Isolate* isolate,
32923257
const std::function<platform::MessageLoopBehavior()>& behavior) {
@@ -3302,17 +3267,12 @@ bool ProcessMessages(
33023267
v8::platform::RunIdleTasks(g_default_platform, isolate,
33033268
50.0 / base::Time::kMillisecondsPerSecond);
33043269
}
3305-
bool ran_finalization_callback = false;
3306-
if (!RunCleanupFinalizationGroupCallback(isolate,
3307-
&ran_finalization_callback)) {
3308-
return false;
3309-
}
33103270
bool ran_set_timeout = false;
33113271
if (!RunSetTimeoutCallback(isolate, &ran_set_timeout)) {
33123272
return false;
33133273
}
33143274

3315-
if (!ran_set_timeout && !ran_finalization_callback) return true;
3275+
if (!ran_set_timeout) return true;
33163276
}
33173277
return true;
33183278
}
@@ -3767,8 +3727,6 @@ int Shell::Main(int argc, char* argv[]) {
37673727
}
37683728

37693729
Isolate* isolate = Isolate::New(create_params);
3770-
isolate->SetHostCleanupFinalizationGroupCallback(
3771-
Shell::HostCleanupFinalizationGroup);
37723730
isolate->SetHostImportModuleDynamicallyCallback(
37733731
Shell::HostImportModuleDynamically);
37743732
isolate->SetHostInitializeImportMetaObjectCallback(
@@ -3831,8 +3789,6 @@ int Shell::Main(int argc, char* argv[]) {
38313789
i::FLAG_hash_seed ^= 1337; // Use a different hash seed.
38323790
Isolate* isolate2 = Isolate::New(create_params);
38333791
i::FLAG_hash_seed ^= 1337; // Restore old hash seed.
3834-
isolate2->SetHostCleanupFinalizationGroupCallback(
3835-
Shell::HostCleanupFinalizationGroup);
38363792
isolate2->SetHostImportModuleDynamicallyCallback(
38373793
Shell::HostImportModuleDynamically);
38383794
isolate2->SetHostInitializeImportMetaObjectCallback(

deps/v8/src/d8/d8.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,6 @@ class PerIsolateData {
226226
PerIsolateData* data_;
227227
};
228228

229-
inline void HostCleanupFinalizationGroup(Local<FinalizationGroup> fg);
230-
inline MaybeLocal<FinalizationGroup> GetCleanupFinalizationGroup();
231229
inline void SetTimeout(Local<Function> callback, Local<Context> context);
232230
inline MaybeLocal<Function> GetTimeoutCallback();
233231
inline MaybeLocal<Context> GetTimeoutContext();
@@ -245,7 +243,6 @@ class PerIsolateData {
245243
Global<Value> realm_shared_;
246244
std::queue<Global<Function>> set_timeout_callbacks_;
247245
std::queue<Global<Context>> set_timeout_contexts_;
248-
std::queue<Global<FinalizationGroup>> cleanup_finalization_groups_;
249246
AsyncHooks* async_hooks_wrapper_;
250247

251248
int RealmIndexOrThrow(const v8::FunctionCallbackInfo<v8::Value>& args,
@@ -423,8 +420,6 @@ class Shell : public i::AllStatic {
423420
static void SetUMask(const v8::FunctionCallbackInfo<v8::Value>& args);
424421
static void MakeDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
425422
static void RemoveDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
426-
static void HostCleanupFinalizationGroup(Local<Context> context,
427-
Local<FinalizationGroup> fg);
428423
static MaybeLocal<Promise> HostImportModuleDynamically(
429424
Local<Context> context, Local<ScriptOrModule> referrer,
430425
Local<String> specifier);

deps/v8/src/execution/isolate.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,6 +1391,10 @@ class Isolate final : private HiddenFactory {
13911391
void ClearKeptObjects();
13921392
void SetHostCleanupFinalizationGroupCallback(
13931393
HostCleanupFinalizationGroupCallback callback);
1394+
HostCleanupFinalizationGroupCallback
1395+
host_cleanup_finalization_group_callback() const {
1396+
return host_cleanup_finalization_group_callback_;
1397+
}
13941398
void RunHostCleanupFinalizationGroupCallback(Handle<JSFinalizationGroup> fg);
13951399

13961400
void SetHostImportModuleDynamicallyCallback(
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright 2020 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "src/heap/finalization-group-cleanup-task.h"
6+
7+
#include "src/execution/frames.h"
8+
#include "src/execution/interrupts-scope.h"
9+
#include "src/execution/stack-guard.h"
10+
#include "src/execution/v8threads.h"
11+
#include "src/heap/heap-inl.h"
12+
#include "src/objects/js-weak-refs-inl.h"
13+
#include "src/tracing/trace-event.h"
14+
15+
namespace v8 {
16+
namespace internal {
17+
18+
FinalizationGroupCleanupTask::FinalizationGroupCleanupTask(Heap* heap)
19+
: heap_(heap) {}
20+
21+
void FinalizationGroupCleanupTask::SlowAssertNoActiveJavaScript() {
22+
#ifdef ENABLE_SLOW_DCHECKS
23+
class NoActiveJavaScript : public ThreadVisitor {
24+
public:
25+
void VisitThread(Isolate* isolate, ThreadLocalTop* top) override {
26+
for (StackFrameIterator it(isolate, top); !it.done(); it.Advance()) {
27+
DCHECK(!it.frame()->is_java_script());
28+
}
29+
}
30+
};
31+
NoActiveJavaScript no_active_js_visitor;
32+
Isolate* isolate = heap_->isolate();
33+
no_active_js_visitor.VisitThread(isolate, isolate->thread_local_top());
34+
isolate->thread_manager()->IterateArchivedThreads(&no_active_js_visitor);
35+
#endif // ENABLE_SLOW_DCHECKS
36+
}
37+
38+
void FinalizationGroupCleanupTask::Run() {
39+
Isolate* isolate = heap_->isolate();
40+
DCHECK(!isolate->host_cleanup_finalization_group_callback());
41+
SlowAssertNoActiveJavaScript();
42+
43+
TRACE_EVENT_CALL_STATS_SCOPED(isolate, "v8",
44+
"V8.FinalizationGroupCleanupTask");
45+
46+
HandleScope handle_scope(isolate);
47+
Handle<JSFinalizationGroup> finalization_group;
48+
// There could be no dirty FinalizationGroups. When a context is disposed by
49+
// the embedder, its FinalizationGroups are removed from the dirty list.
50+
if (!heap_->TakeOneDirtyJSFinalizationGroup().ToHandle(&finalization_group)) {
51+
return;
52+
}
53+
finalization_group->set_scheduled_for_cleanup(false);
54+
55+
// Since FinalizationGroup cleanup callbacks are scheduled by V8, enter the
56+
// FinalizationGroup's context.
57+
Handle<Context> context(Context::cast(finalization_group->native_context()),
58+
isolate);
59+
Handle<Object> callback(finalization_group->cleanup(), isolate);
60+
v8::Context::Scope context_scope(v8::Utils::ToLocal(context));
61+
v8::TryCatch catcher(reinterpret_cast<v8::Isolate*>(isolate));
62+
catcher.SetVerbose(true);
63+
64+
// Exceptions are reported via the message handler. This is ensured by the
65+
// verbose TryCatch.
66+
InvokeFinalizationGroupCleanupFromTask(context, finalization_group, callback);
67+
68+
// Repost if there are remaining dirty FinalizationGroups.
69+
heap_->set_is_finalization_group_cleanup_task_posted(false);
70+
heap_->PostFinalizationGroupCleanupTaskIfNeeded();
71+
}
72+
73+
} // namespace internal
74+
} // namespace v8

0 commit comments

Comments
 (0)