Skip to content

V
V8
Commit df5c72b0 authored by Yang Guo's avatar Yang Guo Committed by Commit Bot
Browse files
Options

Revert "Do not enqueue or run a microtask on detached contexts" 

This reverts commit 734a6575.

Reason for revert: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8-Blink%20Linux%2064/29872

Original change's description:
> Do not enqueue or run a microtask on detached contexts
> 
> This CL disables EnqueueMicrotask and RunMicrotasks on detached
> contexts. That is, if an embedder call DetachGlobal() on a v8::Context,
> EnqueueMicrotask on that context will not take effect, and all Microtask
> that is enqueued before DetachGlobal will be cancelled.
> 
> On Blink, this implies that a frame will no longer run a microtask after
> it's navigated away. OTOH, detached frames in Blink are not affected.
> 
> Bug: v8:8124
> Change-Id: I5b00ceef5ea2afb87cf067a65eb95c29bf91176d
> Reviewed-on: https://chromium-review.googlesource.com/c/1416071
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Adam Klein <adamk@chromium.org>
> Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#59445}

TBR=yukishiino@chromium.org,adamk@chromium.org,yangguo@chromium.org,bmeurer@chromium.org,verwaest@chromium.org,tzik@chromium.org

Change-Id: I9f5b703e7101aa3c251fe03ed4b52e9d71ae605a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8124
Reviewed-on: https://chromium-review.googlesource.com/c/1460466Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59448}
parent f1ab7430
Hide whitespace changes
Inline Side-by-side
Showing with 9 additions and 157 deletions
src/bootstrapper.cc
View file @ df5c72b0
......@@ -353,8 +353,6 @@ void Bootstrapper::DetachGlobal(Handle<Context> env) {
if (FLAG_track_detached_contexts) {
isolate_->AddDetachedContext(env);
}
env->native_context()->set_microtask_queue(nullptr);
}
namespace {
......
src/builtins/builtins-microtask-queue-gen.cc
View file @ df5c72b0
......@@ -34,7 +34,7 @@ class MicrotaskQueueBuiltinsAssembler : public CodeStubAssembler {
TNode<IntPtrT> start,
TNode<IntPtrT> index);
void PrepareForContext(TNode<Context> microtask_context, Label* bailout);
void PrepareForContext(TNode<Context> microtask_context);
void RunSingleMicrotask(TNode<Context> current_context,
TNode<Microtask> microtask);
void IncrementFinishedMicrotaskCount(TNode<RawPtrT> microtask_queue);
......@@ -104,13 +104,8 @@ TNode<IntPtrT> MicrotaskQueueBuiltinsAssembler::CalculateRingBufferOffset(
}
void MicrotaskQueueBuiltinsAssembler::PrepareForContext(
TNode<Context> native_context, Label* bailout) {
TNode<Context> native_context) {
CSA_ASSERT(this, IsNativeContext(native_context));
// Skip the microtask execution if the associated context is shutdown.
GotoIf(WordEqual(GetMicrotaskQueue(native_context), IntPtrConstant(0)),
bailout);
EnterMicrotaskContext(native_context);
SetCurrentContext(native_context);
}
......@@ -155,7 +150,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
TNode<Context> microtask_context =
LoadObjectField<Context>(microtask, CallableTask::kContextOffset);
TNode<Context> native_context = LoadNativeContext(microtask_context);
PrepareForContext(native_context, &done);
PrepareForContext(native_context);
TNode<JSReceiver> callable =
LoadObjectField<JSReceiver>(microtask, CallableTask::kCallableOffset);
......@@ -198,7 +193,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
TNode<Context> microtask_context = LoadObjectField<Context>(
microtask, PromiseResolveThenableJobTask::kContextOffset);
TNode<Context> native_context = LoadNativeContext(microtask_context);
PrepareForContext(native_context, &done);
PrepareForContext(native_context);
Node* const promise_to_resolve = LoadObjectField(
microtask, PromiseResolveThenableJobTask::kPromiseToResolveOffset);
......@@ -222,7 +217,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
TNode<Context> microtask_context = LoadObjectField<Context>(
microtask, PromiseReactionJobTask::kContextOffset);
TNode<Context> native_context = LoadNativeContext(microtask_context);
PrepareForContext(native_context, &done);
PrepareForContext(native_context);
Node* const argument =
LoadObjectField(microtask, PromiseReactionJobTask::kArgumentOffset);
......@@ -255,7 +250,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
TNode<Context> microtask_context = LoadObjectField<Context>(
microtask, PromiseReactionJobTask::kContextOffset);
TNode<Context> native_context = LoadNativeContext(microtask_context);
PrepareForContext(native_context, &done);
PrepareForContext(native_context);
Node* const argument =
LoadObjectField(microtask, PromiseReactionJobTask::kArgumentOffset);
......@@ -291,7 +286,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
FinalizationGroupCleanupJobTask::kFinalizationGroupOffset);
TNode<Context> native_context = LoadObjectField<Context>(
finalization_group, JSFinalizationGroup::kNativeContextOffset);
PrepareForContext(native_context, &done);
PrepareForContext(native_context);
Node* const result = CallRuntime(Runtime::kFinalizationGroupCleanupJob,
native_context, finalization_group);
......@@ -479,11 +474,6 @@ TF_BUILTIN(EnqueueMicrotask, MicrotaskQueueBuiltinsAssembler) {
TNode<Context> native_context = LoadNativeContext(context);
TNode<RawPtrT> microtask_queue = GetMicrotaskQueue(native_context);
// Do not store the microtask if MicrotaskQueue is not available, that may
// happen when the context shutdown.
Label if_shutdown(this, Label::kDeferred);
GotoIf(WordEqual(microtask_queue, IntPtrConstant(0)), &if_shutdown);
TNode<RawPtrT> ring_buffer = GetMicrotaskRingBuffer(microtask_queue);
TNode<IntPtrT> capacity = GetMicrotaskQueueCapacity(microtask_queue);
TNode<IntPtrT> size = GetMicrotaskQueueSize(microtask_queue);
......@@ -516,9 +506,6 @@ TF_BUILTIN(EnqueueMicrotask, MicrotaskQueueBuiltinsAssembler) {
isolate_constant, microtask_queue, microtask);
Return(UndefinedConstant());
}
Bind(&if_shutdown);
Return(UndefinedConstant());
}
TF_BUILTIN(RunMicrotasks, MicrotaskQueueBuiltinsAssembler) {
......
src/objects.h
View file @ df5c72b0
......@@ -941,7 +941,7 @@ class Object {
#ifdef OBJECT_PRINT
// For our gdb macros, we should perhaps change these in the future.
V8_EXPORT_PRIVATE void Print() const;
void Print() const;
// Prints this object with details.
void Print(std::ostream& os) const; // NOLINT
......
test/unittests/microtask-queue-unittest.cc
View file @ df5c72b0
......@@ -10,9 +10,7 @@
#include <vector>
#include "src/heap/factory.h"
#include "src/objects-inl.h"
#include "src/objects/foreign.h"
#include "src/objects/js-array-inl.h"
#include "src/visitors.h"
#include "test/unittests/test-utils.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -27,29 +25,7 @@ void RunStdFunction(void* data) {
(*f)();
}
template <typename TMixin>
class WithFinalizationGroupMixin : public TMixin {
public:
WithFinalizationGroupMixin() {
FLAG_harmony_weak_refs = true;
FLAG_expose_gc = true;
}
private:
SaveFlags save_flags_;
DISALLOW_COPY_AND_ASSIGN(WithFinalizationGroupMixin);
};
using TestWithNativeContextAndFinalizationGroup = //
WithInternalIsolateMixin< //
WithContextMixin< //
WithFinalizationGroupMixin< //
WithIsolateScopeMixin< //
WithSharedIsolateMixin< //
::testing::Test>>>>>;
class MicrotaskQueueTest : public TestWithNativeContextAndFinalizationGroup {
class MicrotaskQueueTest : public TestWithNativeContext {
public:
template <typename F>
Handle<Microtask> NewMicrotask(F&& f) {
......@@ -209,114 +185,5 @@ TEST_F(MicrotaskQueueTest, VisitRoot) {
EXPECT_EQ(expected, actual);
}
TEST_F(MicrotaskQueueTest, DetachGlobal_Enqueue) {
EXPECT_EQ(0, microtask_queue()->size());
// Detach MicrotaskQueue from the current context.
context()->DetachGlobal();
// No microtask should be enqueued after DetachGlobal call.
EXPECT_EQ(0, microtask_queue()->size());
RunJS("Promise.resolve().then(()=>{})");
EXPECT_EQ(0, microtask_queue()->size());
}
TEST_F(MicrotaskQueueTest, DetachGlobal_Run) {
EXPECT_EQ(0, microtask_queue()->size());
// Enqueue microtasks to the current context.
Handle<JSArray> ran = RunJS<JSArray>(
"var ran = [false, false, false, false];"
"Promise.resolve().then(() => { ran[0] = true; });"
"Promise.reject().catch(() => { ran[1] = true; });"
"ran");
Handle<JSFunction> function =
RunJS<JSFunction>("(function() { ran[2] = true; })");
Handle<CallableTask> callable =
factory()->NewCallableTask(function, Utils::OpenHandle(*context()));
microtask_queue()->EnqueueMicrotask(*callable);
// The handler should not run at this point.
const int kNumExpectedTasks = 3;
for (int i = 0; i < kNumExpectedTasks; ++i) {
EXPECT_TRUE(
Object::GetElement(isolate(), ran, i).ToHandleChecked()->IsFalse());
}
EXPECT_EQ(kNumExpectedTasks, microtask_queue()->size());
// Detach MicrotaskQueue from the current context.
context()->DetachGlobal();
// RunMicrotasks processes pending Microtasks, but Microtasks that are
// associated to a detached context should be cancelled and should not take
// effect.
microtask_queue()->RunMicrotasks(isolate());
EXPECT_EQ(0, microtask_queue()->size());
for (int i = 0; i < kNumExpectedTasks; ++i) {
EXPECT_TRUE(
Object::GetElement(isolate(), ran, i).ToHandleChecked()->IsFalse());
}
}
TEST_F(MicrotaskQueueTest, DetachGlobal_FinalizationGroup) {
// Enqueue an FinalizationGroupCleanupTask.
Handle<JSArray> ran = RunJS<JSArray>(
"var ran = [false];"
"var wf = new FinalizationGroup(() => { ran[0] = true; });"
"(function() { wf.register({}, {}); })();"
"gc();"
"ran");
EXPECT_TRUE(
Object::GetElement(isolate(), ran, 0).ToHandleChecked()->IsFalse());
EXPECT_EQ(1, microtask_queue()->size());
// Detach MicrotaskQueue from the current context.
context()->DetachGlobal();
microtask_queue()->RunMicrotasks(isolate());
// RunMicrotasks processes the pending Microtask, but Microtasks that are
// associated to a detached context should be cancelled and should not take
// effect.
EXPECT_EQ(0, microtask_queue()->size());
EXPECT_TRUE(
Object::GetElement(isolate(), ran, 0).ToHandleChecked()->IsFalse());
}
namespace {
void DummyPromiseHook(PromiseHookType type, Local<Promise> promise,
Local<Value> parent) {}
} // namespace
TEST_F(MicrotaskQueueTest, DetachGlobal_PromiseResolveThenableJobTask) {
// Use a PromiseHook to switch the implementation to ResolvePromise runtime,
// instead of ResolvePromise builtin.
v8_isolate()->SetPromiseHook(&DummyPromiseHook);
RunJS(
"var resolve;"
"var promise = new Promise(r => { resolve = r; });"
"promise.then(() => {});"
"resolve({});");
// A PromiseResolveThenableJobTask is pending in the MicrotaskQueue.
EXPECT_EQ(1, microtask_queue()->size());
// Detach MicrotaskQueue from the current context.
context()->DetachGlobal();
// RunMicrotasks processes the pending Microtask, but Microtasks that are
// associated to a detached context should be cancelled and should not take
// effect.
// As PromiseResolveThenableJobTask queues another task for resolution,
// the return value is 2 if it ran.
EXPECT_EQ(1, microtask_queue()->RunMicrotasks(isolate()));
EXPECT_EQ(0, microtask_queue()->size());
}
} // namespace internal
} // namespace v8
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment