Skip to content

Commit

Permalink
deps: V8: cherry-pick 55a01ec7519a
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax authored and BethGriggs committed Apr 20, 2020
1 parent da728c4 commit 1f02617
Show file tree
Hide file tree
Showing 21 changed files with 328 additions and 76 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

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

##### V8 defaults for Node.js #####

Expand Down
2 changes: 2 additions & 0 deletions deps/v8/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2298,6 +2298,8 @@ v8_source_set("v8_base_without_compiler") {
"src/heap/factory-inl.h",
"src/heap/factory.cc",
"src/heap/factory.h",
"src/heap/finalization-group-cleanup-task.cc",
"src/heap/finalization-group-cleanup-task.h",
"src/heap/gc-idle-time-handler.cc",
"src/heap/gc-idle-time-handler.h",
"src/heap/gc-tracer.cc",
Expand Down
14 changes: 10 additions & 4 deletions deps/v8/include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -5885,6 +5885,9 @@ class V8_EXPORT FinalizationGroup : public Object {
* occurred. Otherwise the result is |true| if the cleanup callback
* was called successfully. The result is never |false|.
*/
V8_DEPRECATED(
"FinalizationGroup cleanup is automatic if "
"HostCleanupFinalizationGroupCallback is not set")
static V8_WARN_UNUSED_RESULT Maybe<bool> Cleanup(
Local<FinalizationGroup> finalization_group);
};
Expand Down Expand Up @@ -8444,6 +8447,9 @@ class V8_EXPORT Isolate {
* are ready to be cleaned up and require FinalizationGroup::Cleanup()
* to be called in a future task.
*/
V8_DEPRECATED(
"FinalizationGroup cleanup is automatic if "
"HostCleanupFinalizationGroupCallback is not set")
void SetHostCleanupFinalizationGroupCallback(
HostCleanupFinalizationGroupCallback callback);

Expand Down Expand Up @@ -9056,10 +9062,10 @@ class V8_EXPORT Isolate {
void LowMemoryNotification();

/**
* Optional notification that a context has been disposed. V8 uses
* these notifications to guide the GC heuristic. Returns the number
* of context disposals - including this one - since the last time
* V8 had a chance to clean up.
* Optional notification that a context has been disposed. V8 uses these
* notifications to guide the GC heuristic and cancel FinalizationGroup
* cleanup tasks. Returns the number of context disposals - including this one
* - since the last time V8 had a chance to clean up.
*
* The optional parameter |dependant_context| specifies whether the disposed
* context was depending on state from other contexts or not.
Expand Down
20 changes: 20 additions & 0 deletions deps/v8/src/api/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10950,6 +10950,26 @@ void InvokeFunctionCallback(const v8::FunctionCallbackInfo<v8::Value>& info,
callback(info);
}

void InvokeFinalizationGroupCleanupFromTask(
Handle<Context> context, Handle<JSFinalizationGroup> finalization_group,
Handle<Object> callback) {
Isolate* isolate = finalization_group->native_context().GetIsolate();
RuntimeCallTimerScope timer(
isolate, RuntimeCallCounterId::kFinalizationGroupCleanupFromTask);
// Do not use ENTER_V8 because this is always called from a running
// FinalizationGroupCleanupTask within V8 and we should not log it as an API
// call. This method is implemented here to avoid duplication of the exception
// handling and microtask running logic in CallDepthScope.
if (IsExecutionTerminatingCheck(isolate)) return;
Local<v8::Context> api_context = Utils::ToLocal(context);
CallDepthScope<true> call_depth_scope(isolate, api_context);
VMState<OTHER> state(isolate);
if (JSFinalizationGroup::Cleanup(isolate, finalization_group, callback)
.IsNothing()) {
call_depth_scope.Escape();
}
}

// Undefine macros for jumbo build.
#undef LOG_API
#undef ENTER_V8_DO_NOT_USE
Expand Down
5 changes: 5 additions & 0 deletions deps/v8/src/api/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ namespace v8 {

namespace internal {
class JSArrayBufferView;
class JSFinalizationGroup;
} // namespace internal

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

void InvokeFinalizationGroupCleanupFromTask(
Handle<Context> context, Handle<JSFinalizationGroup> finalization_group,
Handle<Object> callback);

} // namespace internal
} // namespace v8

Expand Down
5 changes: 2 additions & 3 deletions deps/v8/src/builtins/builtins-weak-refs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,8 @@ BUILTIN(FinalizationGroupCleanupSome) {
callback = callback_obj;
}

// Don't do set_scheduled_for_cleanup(false); we still have the microtask
// scheduled and don't want to schedule another one in case the user never
// executes microtasks.
// Don't do set_scheduled_for_cleanup(false); we still have the task
// scheduled.
if (JSFinalizationGroup::Cleanup(isolate, finalization_group, callback)
.IsNothing()) {
DCHECK(isolate->has_pending_exception());
Expand Down
54 changes: 5 additions & 49 deletions deps/v8/src/d8/d8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -916,16 +916,6 @@ MaybeLocal<Promise> Shell::HostImportModuleDynamically(
return MaybeLocal<Promise>();
}

void Shell::HostCleanupFinalizationGroup(Local<Context> context,
Local<FinalizationGroup> fg) {
Isolate* isolate = context->GetIsolate();
PerIsolateData::Get(isolate)->HostCleanupFinalizationGroup(fg);
}

void PerIsolateData::HostCleanupFinalizationGroup(Local<FinalizationGroup> fg) {
cleanup_finalization_groups_.emplace(isolate_, fg);
}

void Shell::HostInitializeImportMetaObject(Local<Context> context,
Local<Module> module,
Local<Object> meta) {
Expand Down Expand Up @@ -1123,15 +1113,6 @@ MaybeLocal<Context> PerIsolateData::GetTimeoutContext() {
return result;
}

MaybeLocal<FinalizationGroup> PerIsolateData::GetCleanupFinalizationGroup() {
if (cleanup_finalization_groups_.empty())
return MaybeLocal<FinalizationGroup>();
Local<FinalizationGroup> result =
cleanup_finalization_groups_.front().Get(isolate_);
cleanup_finalization_groups_.pop();
return result;
}

PerIsolateData::RealmScope::RealmScope(PerIsolateData* data) : data_(data) {
data_->realm_count_ = 1;
data_->realm_current_ = 0;
Expand Down Expand Up @@ -1281,8 +1262,11 @@ void Shell::DisposeRealm(const v8::FunctionCallbackInfo<v8::Value>& args,
int index) {
Isolate* isolate = args.GetIsolate();
PerIsolateData* data = PerIsolateData::Get(isolate);
DisposeModuleEmbedderData(data->realms_[index].Get(isolate));
Local<Context> context = data->realms_[index].Get(isolate);
DisposeModuleEmbedderData(context);
data->realms_[index].Reset();
// ContextDisposedNotification expects the disposed context to be entered.
v8::Context::Scope scope(context);
isolate->ContextDisposedNotification();
isolate->IdleNotificationDeadline(g_platform->MonotonicallyIncreasingTime());
}
Expand Down Expand Up @@ -2742,8 +2726,6 @@ void SourceGroup::ExecuteInThread() {
Isolate::CreateParams create_params;
create_params.array_buffer_allocator = Shell::array_buffer_allocator;
Isolate* isolate = Isolate::New(create_params);
isolate->SetHostCleanupFinalizationGroupCallback(
Shell::HostCleanupFinalizationGroup);
isolate->SetHostImportModuleDynamicallyCallback(
Shell::HostImportModuleDynamically);
isolate->SetHostInitializeImportMetaObjectCallback(
Expand Down Expand Up @@ -2889,8 +2871,6 @@ void Worker::ExecuteInThread() {
Isolate::CreateParams create_params;
create_params.array_buffer_allocator = Shell::array_buffer_allocator;
Isolate* isolate = Isolate::New(create_params);
isolate->SetHostCleanupFinalizationGroupCallback(
Shell::HostCleanupFinalizationGroup);
isolate->SetHostImportModuleDynamicallyCallback(
Shell::HostImportModuleDynamically);
isolate->SetHostInitializeImportMetaObjectCallback(
Expand Down Expand Up @@ -3272,21 +3252,6 @@ bool RunSetTimeoutCallback(Isolate* isolate, bool* did_run) {
return true;
}

bool RunCleanupFinalizationGroupCallback(Isolate* isolate, bool* did_run) {
PerIsolateData* data = PerIsolateData::Get(isolate);
HandleScope handle_scope(isolate);
while (true) {
Local<FinalizationGroup> fg;
if (!data->GetCleanupFinalizationGroup().ToLocal(&fg)) return true;
*did_run = true;
TryCatch try_catch(isolate);
try_catch.SetVerbose(true);
if (FinalizationGroup::Cleanup(fg).IsNothing()) {
return false;
}
}
}

bool ProcessMessages(
Isolate* isolate,
const std::function<platform::MessageLoopBehavior()>& behavior) {
Expand All @@ -3302,17 +3267,12 @@ bool ProcessMessages(
v8::platform::RunIdleTasks(g_default_platform, isolate,
50.0 / base::Time::kMillisecondsPerSecond);
}
bool ran_finalization_callback = false;
if (!RunCleanupFinalizationGroupCallback(isolate,
&ran_finalization_callback)) {
return false;
}
bool ran_set_timeout = false;
if (!RunSetTimeoutCallback(isolate, &ran_set_timeout)) {
return false;
}

if (!ran_set_timeout && !ran_finalization_callback) return true;
if (!ran_set_timeout) return true;
}
return true;
}
Expand Down Expand Up @@ -3767,8 +3727,6 @@ int Shell::Main(int argc, char* argv[]) {
}

Isolate* isolate = Isolate::New(create_params);
isolate->SetHostCleanupFinalizationGroupCallback(
Shell::HostCleanupFinalizationGroup);
isolate->SetHostImportModuleDynamicallyCallback(
Shell::HostImportModuleDynamically);
isolate->SetHostInitializeImportMetaObjectCallback(
Expand Down Expand Up @@ -3831,8 +3789,6 @@ int Shell::Main(int argc, char* argv[]) {
i::FLAG_hash_seed ^= 1337; // Use a different hash seed.
Isolate* isolate2 = Isolate::New(create_params);
i::FLAG_hash_seed ^= 1337; // Restore old hash seed.
isolate2->SetHostCleanupFinalizationGroupCallback(
Shell::HostCleanupFinalizationGroup);
isolate2->SetHostImportModuleDynamicallyCallback(
Shell::HostImportModuleDynamically);
isolate2->SetHostInitializeImportMetaObjectCallback(
Expand Down
5 changes: 0 additions & 5 deletions deps/v8/src/d8/d8.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,6 @@ class PerIsolateData {
PerIsolateData* data_;
};

inline void HostCleanupFinalizationGroup(Local<FinalizationGroup> fg);
inline MaybeLocal<FinalizationGroup> GetCleanupFinalizationGroup();
inline void SetTimeout(Local<Function> callback, Local<Context> context);
inline MaybeLocal<Function> GetTimeoutCallback();
inline MaybeLocal<Context> GetTimeoutContext();
Expand All @@ -245,7 +243,6 @@ class PerIsolateData {
Global<Value> realm_shared_;
std::queue<Global<Function>> set_timeout_callbacks_;
std::queue<Global<Context>> set_timeout_contexts_;
std::queue<Global<FinalizationGroup>> cleanup_finalization_groups_;
AsyncHooks* async_hooks_wrapper_;

int RealmIndexOrThrow(const v8::FunctionCallbackInfo<v8::Value>& args,
Expand Down Expand Up @@ -423,8 +420,6 @@ class Shell : public i::AllStatic {
static void SetUMask(const v8::FunctionCallbackInfo<v8::Value>& args);
static void MakeDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
static void RemoveDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
static void HostCleanupFinalizationGroup(Local<Context> context,
Local<FinalizationGroup> fg);
static MaybeLocal<Promise> HostImportModuleDynamically(
Local<Context> context, Local<ScriptOrModule> referrer,
Local<String> specifier);
Expand Down
4 changes: 4 additions & 0 deletions deps/v8/src/execution/isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,10 @@ class Isolate final : private HiddenFactory {
void ClearKeptObjects();
void SetHostCleanupFinalizationGroupCallback(
HostCleanupFinalizationGroupCallback callback);
HostCleanupFinalizationGroupCallback
host_cleanup_finalization_group_callback() const {
return host_cleanup_finalization_group_callback_;
}
void RunHostCleanupFinalizationGroupCallback(Handle<JSFinalizationGroup> fg);

void SetHostImportModuleDynamicallyCallback(
Expand Down
74 changes: 74 additions & 0 deletions deps/v8/src/heap/finalization-group-cleanup-task.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright 2020 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "src/heap/finalization-group-cleanup-task.h"

#include "src/execution/frames.h"
#include "src/execution/interrupts-scope.h"
#include "src/execution/stack-guard.h"
#include "src/execution/v8threads.h"
#include "src/heap/heap-inl.h"
#include "src/objects/js-weak-refs-inl.h"
#include "src/tracing/trace-event.h"

namespace v8 {
namespace internal {

FinalizationGroupCleanupTask::FinalizationGroupCleanupTask(Heap* heap)
: heap_(heap) {}

void FinalizationGroupCleanupTask::SlowAssertNoActiveJavaScript() {
#ifdef ENABLE_SLOW_DCHECKS
class NoActiveJavaScript : public ThreadVisitor {
public:
void VisitThread(Isolate* isolate, ThreadLocalTop* top) override {
for (StackFrameIterator it(isolate, top); !it.done(); it.Advance()) {
DCHECK(!it.frame()->is_java_script());
}
}
};
NoActiveJavaScript no_active_js_visitor;
Isolate* isolate = heap_->isolate();
no_active_js_visitor.VisitThread(isolate, isolate->thread_local_top());
isolate->thread_manager()->IterateArchivedThreads(&no_active_js_visitor);
#endif // ENABLE_SLOW_DCHECKS
}

void FinalizationGroupCleanupTask::Run() {
Isolate* isolate = heap_->isolate();
DCHECK(!isolate->host_cleanup_finalization_group_callback());
SlowAssertNoActiveJavaScript();

TRACE_EVENT_CALL_STATS_SCOPED(isolate, "v8",
"V8.FinalizationGroupCleanupTask");

HandleScope handle_scope(isolate);
Handle<JSFinalizationGroup> finalization_group;
// There could be no dirty FinalizationGroups. When a context is disposed by
// the embedder, its FinalizationGroups are removed from the dirty list.
if (!heap_->TakeOneDirtyJSFinalizationGroup().ToHandle(&finalization_group)) {
return;
}
finalization_group->set_scheduled_for_cleanup(false);

// Since FinalizationGroup cleanup callbacks are scheduled by V8, enter the
// FinalizationGroup's context.
Handle<Context> context(Context::cast(finalization_group->native_context()),
isolate);
Handle<Object> callback(finalization_group->cleanup(), isolate);
v8::Context::Scope context_scope(v8::Utils::ToLocal(context));
v8::TryCatch catcher(reinterpret_cast<v8::Isolate*>(isolate));
catcher.SetVerbose(true);

// Exceptions are reported via the message handler. This is ensured by the
// verbose TryCatch.
InvokeFinalizationGroupCleanupFromTask(context, finalization_group, callback);

// Repost if there are remaining dirty FinalizationGroups.
heap_->set_is_finalization_group_cleanup_task_posted(false);
heap_->PostFinalizationGroupCleanupTaskIfNeeded();
}

} // namespace internal
} // namespace v8
36 changes: 36 additions & 0 deletions deps/v8/src/heap/finalization-group-cleanup-task.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2020 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_
#define V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_

#include "include/v8-platform.h"
#include "src/objects/js-weak-refs.h"

namespace v8 {
namespace internal {

// The GC schedules a cleanup task when the dirty FinalizationGroup list is
// non-empty. The task processes a single FinalizationGroup and posts another
// cleanup task if there are remaining dirty FinalizationGroups on the list.
class FinalizationGroupCleanupTask : public Task {
public:
explicit FinalizationGroupCleanupTask(Heap* heap);
~FinalizationGroupCleanupTask() override = default;

void Run() override;

private:
FinalizationGroupCleanupTask(const FinalizationGroupCleanupTask&) = delete;
void operator=(const FinalizationGroupCleanupTask&) = delete;

void SlowAssertNoActiveJavaScript();

Heap* heap_;
};

} // namespace internal
} // namespace v8

#endif // V8_HEAP_FINALIZATION_GROUP_CLEANUP_TASK_H_
4 changes: 4 additions & 0 deletions deps/v8/src/heap/heap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,10 @@ void Heap::DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
base::CheckedDecrement(&backing_store_bytes_, amount);
}

bool Heap::HasDirtyJSFinalizationGroups() {
return !dirty_js_finalization_groups().IsUndefined(isolate());
}

AlwaysAllocateScope::AlwaysAllocateScope(Heap* heap) : heap_(heap) {
heap_->always_allocate_scope_count_++;
}
Expand Down
Loading

0 comments on commit 1f02617

Please sign in to comment.