Skip to content

Commit 55a01ec

Browse files
sygCommit Bot
syg
authored and
Commit Bot
committed
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 31d8ff7 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}
1 parent 841fd70 commit 55a01ec

20 files changed

Lines changed: 328 additions & 75 deletions

‎BUILD.gn‎

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

‎include/v8.h‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5912,6 +5912,9 @@ class V8_EXPORT FinalizationGroup : public Object {
59125912
* occurred. Otherwise the result is |true| if the cleanup callback
59135913
* was called successfully. The result is never |false|.
59145914
*/
5915+
V8_DEPRECATED(
5916+
"FinalizationGroup cleanup is automatic if "
5917+
"HostCleanupFinalizationGroupCallback is not set")
59155918
static V8_WARN_UNUSED_RESULT Maybe<bool> Cleanup(
59165919
Local<FinalizationGroup> finalization_group);
59175920
};
@@ -8481,6 +8484,9 @@ class V8_EXPORT Isolate {
84818484
* are ready to be cleaned up and require FinalizationGroup::Cleanup()
84828485
* to be called in a future task.
84838486
*/
8487+
V8_DEPRECATED(
8488+
"FinalizationGroup cleanup is automatic if "
8489+
"HostCleanupFinalizationGroupCallback is not set")
84848490
void SetHostCleanupFinalizationGroupCallback(
84858491
HostCleanupFinalizationGroupCallback callback);
84868492

@@ -9085,10 +9091,10 @@ class V8_EXPORT Isolate {
90859091
void LowMemoryNotification();
90869092

90879093
/**
9088-
* Optional notification that a context has been disposed. V8 uses
9089-
* these notifications to guide the GC heuristic. Returns the number
9090-
* of context disposals - including this one - since the last time
9091-
* V8 had a chance to clean up.
9094+
* Optional notification that a context has been disposed. V8 uses these
9095+
* notifications to guide the GC heuristic and cancel FinalizationGroup
9096+
* cleanup tasks. Returns the number of context disposals - including this one
9097+
* - since the last time V8 had a chance to clean up.
90929098
*
90939099
* The optional parameter |dependant_context| specifies whether the disposed
90949100
* context was depending on state from other contexts or not.

‎src/api/api.cc‎

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

11036+
void InvokeFinalizationGroupCleanupFromTask(
11037+
Handle<Context> context, Handle<JSFinalizationGroup> finalization_group,
11038+
Handle<Object> callback) {
11039+
Isolate* isolate = finalization_group->native_context().GetIsolate();
11040+
RuntimeCallTimerScope timer(
11041+
isolate, RuntimeCallCounterId::kFinalizationGroupCleanupFromTask);
11042+
// Do not use ENTER_V8 because this is always called from a running
11043+
// FinalizationGroupCleanupTask within V8 and we should not log it as an API
11044+
// call. This method is implemented here to avoid duplication of the exception
11045+
// handling and microtask running logic in CallDepthScope.
11046+
if (IsExecutionTerminatingCheck(isolate)) return;
11047+
Local<v8::Context> api_context = Utils::ToLocal(context);
11048+
CallDepthScope<true> call_depth_scope(isolate, api_context);
11049+
VMState<OTHER> state(isolate);
11050+
if (JSFinalizationGroup::Cleanup(isolate, finalization_group, callback)
11051+
.IsNothing()) {
11052+
call_depth_scope.Escape();
11053+
}
11054+
}
11055+
1103611056
// Undefine macros for jumbo build.
1103711057
#undef LOG_API
1103811058
#undef ENTER_V8_DO_NOT_USE

‎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

‎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());

‎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) {
@@ -3303,17 +3268,12 @@ bool ProcessMessages(
33033268
v8::platform::RunIdleTasks(g_default_platform, isolate,
33043269
50.0 / base::Time::kMillisecondsPerSecond);
33053270
}
3306-
bool ran_finalization_callback = false;
3307-
if (!RunCleanupFinalizationGroupCallback(isolate,
3308-
&ran_finalization_callback)) {
3309-
return false;
3310-
}
33113271
bool ran_set_timeout = false;
33123272
if (!RunSetTimeoutCallback(isolate, &ran_set_timeout)) {
33133273
return false;
33143274
}
33153275

3316-
if (!ran_set_timeout && !ran_finalization_callback) return true;
3276+
if (!ran_set_timeout) return true;
33173277
}
33183278
return true;
33193279
}
@@ -3768,8 +3728,6 @@ int Shell::Main(int argc, char* argv[]) {
37683728
}
37693729

37703730
Isolate* isolate = Isolate::New(create_params);
3771-
isolate->SetHostCleanupFinalizationGroupCallback(
3772-
Shell::HostCleanupFinalizationGroup);
37733731
isolate->SetHostImportModuleDynamicallyCallback(
37743732
Shell::HostImportModuleDynamically);
37753733
isolate->SetHostInitializeImportMetaObjectCallback(
@@ -3832,8 +3790,6 @@ int Shell::Main(int argc, char* argv[]) {
38323790
i::FLAG_hash_seed ^= 1337; // Use a different hash seed.
38333791
Isolate* isolate2 = Isolate::New(create_params);
38343792
i::FLAG_hash_seed ^= 1337; // Restore old hash seed.
3835-
isolate2->SetHostCleanupFinalizationGroupCallback(
3836-
Shell::HostCleanupFinalizationGroup);
38373793
isolate2->SetHostImportModuleDynamicallyCallback(
38383794
Shell::HostImportModuleDynamically);
38393795
isolate2->SetHostInitializeImportMetaObjectCallback(

‎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);

‎src/execution/isolate.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,6 +1404,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
14041404
void ClearKeptObjects();
14051405
void SetHostCleanupFinalizationGroupCallback(
14061406
HostCleanupFinalizationGroupCallback callback);
1407+
HostCleanupFinalizationGroupCallback
1408+
host_cleanup_finalization_group_callback() const {
1409+
return host_cleanup_finalization_group_callback_;
1410+
}
14071411
void RunHostCleanupFinalizationGroupCallback(Handle<JSFinalizationGroup> fg);
14081412

14091413
void SetHostImportModuleDynamicallyCallback(

‎src/heap/finalization-group-cleanup-task.cc‎

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

‎src/heap/finalization-group-cleanup-task.h‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
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+
#ifndef V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_
6+
#define V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_
7+
8+
#include "include/v8-platform.h"
9+
#include "src/objects/js-weak-refs.h"
10+
11+
namespace v8 {
12+
namespace internal {
13+
14+
// The GC schedules a cleanup task when the dirty FinalizationGroup list is
15+
// non-empty. The task processes a single FinalizationGroup and posts another
16+
// cleanup task if there are remaining dirty FinalizationGroups on the list.
17+
class FinalizationGroupCleanupTask : public Task {
18+
public:
19+
explicit FinalizationGroupCleanupTask(Heap* heap);
20+
~FinalizationGroupCleanupTask() override = default;
21+
22+
void Run() override;
23+
24+
private:
25+
FinalizationGroupCleanupTask(const FinalizationGroupCleanupTask&) = delete;
26+
void operator=(const FinalizationGroupCleanupTask&) = delete;
27+
28+
void SlowAssertNoActiveJavaScript();
29+
30+
Heap* heap_;
31+
};
32+
33+
} // namespace internal
34+
} // namespace v8
35+
36+
#endif // V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_

0 commit comments

Comments
 (0)