Commit 80ec52ea authored by Omer Katz's avatar Omer Katz Committed by Commit Bot

cppgc: Don't override stack_state in unified heap GC finalization.

CppHeap is currently set up to always finalize with no stack.
Finalizing with actual current stack state breaks our unified heap
unittests. This is fixed by having test specify which stack state
to pass CppHeap.

Bug: chromium:1056170
Change-Id: I1a6c3870abbdf56917c20c6a75580b6c516d828c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2494924
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70729}
parent 1debc0ca
...@@ -194,7 +194,7 @@ bool CppHeap::AdvanceTracing(double deadline_in_ms) { ...@@ -194,7 +194,7 @@ bool CppHeap::AdvanceTracing(double deadline_in_ms) {
bool CppHeap::IsTracingDone() { return marking_done_; } bool CppHeap::IsTracingDone() { return marking_done_; }
void CppHeap::EnterFinalPause(EmbedderStackState stack_state) { void CppHeap::EnterFinalPause(EmbedderStackState stack_state) {
marker_->EnterAtomicPause(cppgc::Heap::StackState::kNoHeapPointers); marker_->EnterAtomicPause(stack_state);
} }
void CppHeap::TraceEpilogue(TraceSummary* trace_summary) { void CppHeap::TraceEpilogue(TraceSummary* trace_summary) {
......
...@@ -1127,7 +1127,7 @@ class Heap { ...@@ -1127,7 +1127,7 @@ class Heap {
EmbedderHeapTracer* GetEmbedderHeapTracer() const; EmbedderHeapTracer* GetEmbedderHeapTracer() const;
void RegisterExternallyReferencedObject(Address* location); void RegisterExternallyReferencedObject(Address* location);
void SetEmbedderStackStateForNextFinalizaton( V8_EXPORT_PRIVATE void SetEmbedderStackStateForNextFinalizaton(
EmbedderHeapTracer::EmbedderStackState stack_state); EmbedderHeapTracer::EmbedderStackState stack_state);
EmbedderHeapTracer::TraceFlags flags_for_embedder_tracer() const; EmbedderHeapTracer::TraceFlags flags_for_embedder_tracer() const;
......
...@@ -56,6 +56,18 @@ class UnifiedHeapTest : public TestWithHeapInternals { ...@@ -56,6 +56,18 @@ class UnifiedHeapTest : public TestWithHeapInternals {
cppgc::ShutdownProcess(); cppgc::ShutdownProcess();
} }
void CollectGarbageWithEmbedderStack() {
heap()->SetEmbedderStackStateForNextFinalizaton(
EmbedderHeapTracer::EmbedderStackState::kMayContainHeapPointers);
CollectGarbage(OLD_SPACE);
}
void CollectGarbageWithoutEmbedderStack() {
heap()->SetEmbedderStackStateForNextFinalizaton(
EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
CollectGarbage(OLD_SPACE);
}
CppHeap& cpp_heap() const { return *cpp_heap_.get(); } CppHeap& cpp_heap() const { return *cpp_heap_.get(); }
cppgc::AllocationHandle& allocation_handle() { cppgc::AllocationHandle& allocation_handle() {
...@@ -80,7 +92,7 @@ size_t Wrappable::destructor_callcount = 0; ...@@ -80,7 +92,7 @@ size_t Wrappable::destructor_callcount = 0;
} // namespace } // namespace
TEST_F(UnifiedHeapTest, OnlyGC) { CollectGarbage(OLD_SPACE); } TEST_F(UnifiedHeapTest, OnlyGC) { CollectGarbageWithEmbedderStack(); }
TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) { TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) {
v8::HandleScope scope(v8_isolate()); v8::HandleScope scope(v8_isolate());
...@@ -90,12 +102,12 @@ TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) { ...@@ -90,12 +102,12 @@ TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) {
context, cppgc::MakeGarbageCollected<Wrappable>(allocation_handle())); context, cppgc::MakeGarbageCollected<Wrappable>(allocation_handle()));
EXPECT_FALSE(api_object.IsEmpty()); EXPECT_FALSE(api_object.IsEmpty());
EXPECT_EQ(0u, Wrappable::destructor_callcount); EXPECT_EQ(0u, Wrappable::destructor_callcount);
CollectGarbage(OLD_SPACE); CollectGarbageWithoutEmbedderStack();
EXPECT_EQ(0u, Wrappable::destructor_callcount); EXPECT_EQ(0u, Wrappable::destructor_callcount);
ResetWrappableConnection(api_object); ResetWrappableConnection(api_object);
CollectGarbage(OLD_SPACE); CollectGarbageWithoutEmbedderStack();
// Calling CollectGarbage twice to force the first GC to finish sweeping. // Calling CollectGarbage twice to force the first GC to finish sweeping.
CollectGarbage(OLD_SPACE); CollectGarbageWithoutEmbedderStack();
EXPECT_EQ(1u, Wrappable::destructor_callcount); EXPECT_EQ(1u, Wrappable::destructor_callcount);
} }
......
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