diff options
Diffstat (limited to 'chromium/third_party/blink/renderer/platform/heap')
28 files changed, 1419 insertions, 1313 deletions
diff --git a/chromium/third_party/blink/renderer/platform/heap/BUILD.gn b/chromium/third_party/blink/renderer/platform/heap/BUILD.gn index 470d7033ba4..197d649e9ff 100644 --- a/chromium/third_party/blink/renderer/platform/heap/BUILD.gn +++ b/chromium/third_party/blink/renderer/platform/heap/BUILD.gn @@ -32,6 +32,7 @@ blink_platform_sources("heap") { sources = [ "address_cache.cc", "address_cache.h", + "atomic_entry_flag.h", "blink_gc.h", "blink_gc_memory_dump_provider.cc", "blink_gc_memory_dump_provider.h", @@ -119,6 +120,7 @@ jumbo_source_set("blink_heap_unittests_sources") { "heap_test.cc", "heap_test_utilities.cc", "heap_test_utilities.h", + "heap_thread_test.cc", "heap_traits_test.cc", "incremental_marking_test.cc", "name_trait_test.cc", diff --git a/chromium/third_party/blink/renderer/platform/heap/BlinkGCAPIReference.md b/chromium/third_party/blink/renderer/platform/heap/BlinkGCAPIReference.md index 389b9b730f0..880f6b4db05 100644 --- a/chromium/third_party/blink/renderer/platform/heap/BlinkGCAPIReference.md +++ b/chromium/third_party/blink/renderer/platform/heap/BlinkGCAPIReference.md @@ -25,7 +25,7 @@ A class that wants the lifetime management of its instances to be managed by Bli ```c++ class YourClass : public GarbageCollected<YourClass> { - // ... + // ... }; ``` @@ -68,10 +68,10 @@ void someFunction(P*); class A : public GarbageCollected<A>, public P { public: - void someMemberFunction() - { - someFunction(this); // DANGEROUS, a raw pointer to an on-heap object. - } + void someMemberFunction() + { + someFunction(this); // DANGEROUS, a raw pointer to an on-heap object. + } }; ``` @@ -88,10 +88,10 @@ A class is said to *need finalization* when it meets either of the following cri ```c++ class YourClass : public GarbageCollectedFinalized<YourClass> { public: - ~YourClass() { ... } // Non-empty destructor means finalization is needed. + ~YourClass() { ... } // Non-empty destructor means finalization is needed. private: - scoped_refptr<Something> m_something; // scoped_refptr<> has non-empty destructor, so finalization is needed. + scoped_refptr<Something> something_; // scoped_refptr<> has non-empty destructor, so finalization is needed. }; ``` @@ -146,17 +146,17 @@ class Q : public GarbageCollectedMixin { }; class R : public Q { }; class A : public GarbageCollected<A>, public P, public R { - USING_GARBAGE_COLLECTED_MIXIN(A); // OK, resolving pure virtual functions of P and R. + USING_GARBAGE_COLLECTED_MIXIN(A); // OK, resolving pure virtual functions of P and R. }; class B : public GarbageCollected<B>, public P { - USING_GARBAGE_COLLECTED_MIXIN(B); // OK, different garbage-collected classes may inherit from the same mixin (P). + USING_GARBAGE_COLLECTED_MIXIN(B); // OK, different garbage-collected classes may inherit from the same mixin (P). }; void someFunction() { - new A; // OK, A can be instantiated. - // new R; // BAD, R has pure virtual functions. + MakeGarbageCollected<A>(); // OK, A can be instantiated. + // MakeGarbageCollected<R>(); // BAD, R has pure virtual functions. } ``` @@ -183,19 +183,19 @@ A pre-finalizer must have the following function signature: `void preFinalizer() ```c++ class YourClass : public GarbageCollectedFinalized<YourClass> { - USING_PRE_FINALIZER(YourClass, dispose); + USING_PRE_FINALIZER(YourClass, dispose); public: - void dispose() - { - m_other->dispose(); // OK; you can touch other on-heap objects in a pre-finalizer. - } - ~YourClass() - { - // m_other->dispose(); // BAD. - } + void dispose() + { + other_->dispose(); // OK; you can touch other on-heap objects in a pre-finalizer. + } + ~YourClass() + { + // other_->dispose(); // BAD. + } private: - Member<OtherClass> m_other; + Member<OtherClass> other_; }; ``` @@ -254,8 +254,8 @@ On-stack references to on-heap objects must be raw pointers. ```c++ void someFunction() { - SomeGarbageCollectedClass* object = new SomeGarbageCollectedClass; // OK, retained by a pointer. - ... + SomeGarbageCollectedClass* object = MakeGarbageCollected<SomeGarbageCollectedClass>(); // OK, retained by a pointer. + ... } // OK to leave the object behind. The Blink GC system will free it up when it becomes unused. ``` @@ -276,10 +276,10 @@ because this rewrite is only done within Blink GC's garbage collection period. ```c++ class SomeGarbageCollectedClass : public GarbageCollected<GarbageCollectedSomething> { - ... + ... private: - Member<AnotherGarbageCollectedClass> m_another; // OK, retained by Member<T>. - WeakMember<AnotherGarbageCollectedClass> m_anotherWeak; // OK, weak reference. + Member<AnotherGarbageCollectedClass> another_; // OK, retained by Member<T>. + WeakMember<AnotherGarbageCollectedClass> anotherWeak_; // OK, weak reference. }; ``` @@ -309,9 +309,9 @@ garbage-collected, just like `WeakMember<T>`. #include "third_party/blink/renderer/platform/heap/persistent.h" ... class NonGarbageCollectedClass { - ... + ... private: - Persistent<SomeGarbageCollectedClass> m_something; // OK, the object will be alive while this persistent is alive. + Persistent<SomeGarbageCollectedClass> something_; // OK, the object will be alive while this persistent is alive. }; ``` @@ -399,10 +399,10 @@ class C : public B { }; void C::Trace(Visitor* visitor) { - visitor->Trace(x_); - visitor->Trace(y_); // Weak member needs to be traced. - visitor->Trace(z_); // Heap collection does, too. - B::Trace(visitor); // Delegate to the parent. In this case it's empty, but this is required. + visitor->Trace(x_); + visitor->Trace(y_); // Weak member needs to be traced. + visitor->Trace(z_); // Heap collection does, too. + B::Trace(visitor); // Delegate to the parent. In this case it's empty, but this is required. } ``` @@ -414,26 +414,26 @@ phase: void C::ClearWeakMembers(Visitor* visitor) { - if (ThreadHeap::isHeapObjectAlive(y_)) - return; - - // |m_y| is not referred to by anyone else, clear the weak - // reference along with updating state / clearing any other - // resources at the same time. None of those operations are - // allowed to perform heap allocations: - y_->detach(); - - // Note: if the weak callback merely clears the weak reference, - // it is much simpler to just |trace| the field rather than - // install a custom weak callback. - y_ = nullptr; + if (ThreadHeap::isHeapObjectAlive(y_)) + return; + + // |y_| is not referred to by anyone else, clear the weak + // reference along with updating state / clearing any other + // resources at the same time. None of those operations are + // allowed to perform heap allocations: + y_->detach(); + + // Note: if the weak callback merely clears the weak reference, + // it is much simpler to just |trace| the field rather than + // install a custom weak callback. + y_ = nullptr; } void C::Trace(Visitor* visitor) { - visitor->template registerWeakMembers<C, &C::ClearWeakMembers>(this); - visitor->Trace(x_); - visitor->Trace(z_); // Heap collection does, too. - B::Trace(visitor); // Delegate to the parent. In this case it's empty, but this is required. + visitor->template registerWeakMembers<C, &C::ClearWeakMembers>(this); + visitor->Trace(x_); + visitor->Trace(z_); // Heap collection does, too. + B::Trace(visitor); // Delegate to the parent. In this case it's empty, but this is required. } ``` diff --git a/chromium/third_party/blink/renderer/platform/heap/atomic_entry_flag.h b/chromium/third_party/blink/renderer/platform/heap/atomic_entry_flag.h new file mode 100644 index 00000000000..0404e9e4c70 --- /dev/null +++ b/chromium/third_party/blink/renderer/platform/heap/atomic_entry_flag.h @@ -0,0 +1,47 @@ +// Copyright 2018 The Chromium 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 THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_ATOMIC_ENTRY_FLAG_H_ +#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_ATOMIC_ENTRY_FLAG_H_ + +#include <atomic> + +namespace blink { + +// A flag which provides a fast check whether a scope may be entered on the +// current thread, without needing to access thread-local storage or mutex. +// +// Can have false positives (i.e., spuriously report that it might be entered), +// so it is expected that this will be used in tandem with a precise check that +// the scope is in fact entered on that thread. +// +// Example: +// g_frobnicating_flag.MightBeEntered() && +// ThreadLocalFrobnicator().IsFrobnicating() +// +// Relaxed atomic operations are sufficient, since: +// - all accesses remain atomic +// - each thread must observe its own operations in order +// - no thread ever exits the flag more times than it enters (if used correctly) +// And so if a thread observes zero, it must be because it has observed an equal +// number of exits as entries. +class AtomicEntryFlag { + public: + inline void Enter() { entries_.fetch_add(1, std::memory_order_relaxed); } + inline void Exit() { entries_.fetch_sub(1, std::memory_order_relaxed); } + + // Returns false only if the current thread is not between a call to Enter and + // a call to Exit. Returns true if this thread or another thread may currently + // be in the scope guarded by this flag. + inline bool MightBeEntered() const { + return entries_.load(std::memory_order_relaxed) != 0; + } + + private: + std::atomic_int entries_{0}; +}; + +} // namespace blink + +#endif // THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_ATOMIC_ENTRY_FLAG_H_ diff --git a/chromium/third_party/blink/renderer/platform/heap/blink_gc.h b/chromium/third_party/blink/renderer/platform/heap/blink_gc.h index 04dac693e11..bc35c3519ce 100644 --- a/chromium/third_party/blink/renderer/platform/heap/blink_gc.h +++ b/chromium/third_party/blink/renderer/platform/heap/blink_gc.h @@ -14,7 +14,6 @@ namespace blink { -class HeapObjectHeader; class MarkingVisitor; class Visitor; @@ -28,12 +27,6 @@ using WeakCallback = VisitorCallback; using EphemeronCallback = VisitorCallback; using NameCallback = const char* (*)(const void* self); -// Callback used for unit testing the marking of conservative pointers -// (|CheckAndMarkPointer|). For each pointer that has been discovered to point -// to a heap object, the callback is invoked with a pointer to its header. If -// the callback returns true, the object will not be marked. -using MarkedPointerCallbackForTesting = bool (*)(HeapObjectHeader*); - // Simple alias to avoid heap compaction type signatures turning into // a sea of generic |void*|s. using MovableReference = void*; diff --git a/chromium/third_party/blink/renderer/platform/heap/garbage_collected.h b/chromium/third_party/blink/renderer/platform/heap/garbage_collected.h index d07e1e4cf23..2ccb869c823 100644 --- a/chromium/third_party/blink/renderer/platform/heap/garbage_collected.h +++ b/chromium/third_party/blink/renderer/platform/heap/garbage_collected.h @@ -47,12 +47,17 @@ struct IsGarbageCollectedMixin { static const bool value = sizeof(CheckMarker<T>(nullptr)) == sizeof(YesType); }; +// TraceDescriptor is used to describe how to trace an object. struct TraceDescriptor { STACK_ALLOCATED(); public: + // The adjusted base pointer of the object that should be traced. void* base_object_payload; + // A callback for tracing the object. TraceCallback callback; + // Indicator whether this object can be traced recursively or whether it + // requires iterative tracing. bool can_trace_eagerly; }; @@ -72,11 +77,14 @@ struct TraceDescriptor { // USING_GARBAGE_COLLECTED_MIXIN(A); // }; // -// With the helper, as long as we are using Member<B>, TypeTrait<B> will -// dispatch dynamically to retrieve the necessary tracing and header methods. -// Note that this is only enabled for Member<B>. For Member<A> which we can -// compute the necessary methods and pointers statically and this dynamic -// dispatch is not used. +// The classes involved and the helper macros allow for properly handling +// definitions for Member<B> and friends. The mechanisms for handling Member<B> +// involve dynamic dispatch. Note that for Member<A> all methods and pointers +// are statically computed and no dynamic dispatch is involved. +// +// Note that garbage collections are allowed during mixin construction as +// conservative scanning of objects does not rely on the Trace method but rather +// scans the object field by field. class PLATFORM_EXPORT GarbageCollectedMixin { public: typedef int IsGarbageCollectedMixinMarker; @@ -86,7 +94,8 @@ class PLATFORM_EXPORT GarbageCollectedMixin { // these objects can processed later on. This is necessary as // not-fully-constructed mixin objects potentially require being processed // as part emitting a write barrier for incremental marking. See - // IncrementalMarkingTest::WriteBarrierDuringMixinConstruction as an example. + // |IncrementalMarkingTest::WriteBarrierDuringMixinConstruction| as an + // example. // // The not-fully-constructed objects are handled as follows: // 1. Write barrier or marking of not fully constructed mixin gets called. @@ -127,114 +136,48 @@ class PLATFORM_EXPORT GarbageCollectedMixin { \ private: -// A C++ object's vptr will be initialized to its leftmost base's vtable after -// the constructors of all its subclasses have run, so if a subclass constructor -// tries to access any of the vtbl entries of its leftmost base prematurely, -// it'll find an as-yet incorrect vptr and fail. Which is exactly what a -// garbage collector will try to do if it tries to access the leftmost base -// while one of the subclass constructors of a GC mixin object triggers a GC. -// It is consequently not safe to allow any GCs while these objects are under -// (sub constructor) construction. -// -// To prevent GCs in that restricted window of a mixin object's construction: -// -// - The initial allocation of the mixin object will enter a no GC scope. -// This is done by overriding 'operator new' for mixin instances. -// - When the constructor for the mixin is invoked, after all the -// derived constructors have run, it will invoke the constructor -// for a field whose only purpose is to leave the GC scope. -// GarbageCollectedMixinConstructorMarker's constructor takes care of -// this and the field is declared by way of USING_GARBAGE_COLLECTED_MIXIN(). - -#define DEFINE_GARBAGE_COLLECTED_MIXIN_CONSTRUCTOR_MARKER(TYPE) \ - public: \ - GC_PLUGIN_IGNORE("crbug.com/456823") \ - NO_SANITIZE_UNRELATED_CAST void* operator new(size_t size) { \ - CHECK_GE(kLargeObjectSizeThreshold, size) \ - << "GarbageCollectedMixin may not be a large object"; \ - void* object = \ - TYPE::AllocateObject(size, IsEagerlyFinalizedType<TYPE>::value); \ - ThreadState* state = \ - ThreadStateFor<ThreadingTrait<TYPE>::kAffinity>::GetState(); \ - state->EnterGCForbiddenScopeIfNeeded( \ - &(reinterpret_cast<TYPE*>(object)->mixin_constructor_marker_)); \ - return object; \ - } \ - GarbageCollectedMixinConstructorMarker<ThreadingTrait<TYPE>::kAffinity> \ - mixin_constructor_marker_; \ - \ +// The Oilpan GC plugin checks for proper usages of the +// USING_GARBAGE_COLLECTED_MIXIN macro using a typedef marker. +#define DEFINE_GARBAGE_COLLECTED_MIXIN_CONSTRUCTOR_MARKER(TYPE) \ + public: \ + typedef int HasUsingGarbageCollectedMixinMacro; \ + \ private: -// Mixins that wrap/nest others requires extra handling: -// -// class A : public GarbageCollected<A>, public GarbageCollectedMixin { -// USING_GARBAGE_COLLECTED_MIXIN(A); -// ... -// }' -// public B final : public A, public SomeOtherMixinInterface { -// USING_GARBAGE_COLLECTED_MIXIN(B); -// ... -// }; -// -// The "operator new" for B will enter the forbidden GC scope, but -// upon construction, two GarbageCollectedMixinConstructorMarker constructors -// will run -- one for A (first) and another for B (secondly). Only -// the second one should leave the forbidden GC scope. This is realized by -// recording the address of B's GarbageCollectedMixinConstructorMarker -// when the "operator new" for B runs, and leaving the forbidden GC scope -// when the constructor of the recorded GarbageCollectedMixinConstructorMarker -// runs. +// The USING_GARBAGE_COLLECTED_MIXIN macro defines all methods and markers +// needed for handling mixins. #define USING_GARBAGE_COLLECTED_MIXIN(TYPE) \ IS_GARBAGE_COLLECTED_TYPE(); \ DEFINE_GARBAGE_COLLECTED_MIXIN_METHODS(TYPE) \ DEFINE_GARBAGE_COLLECTED_MIXIN_CONSTRUCTOR_MARKER(TYPE) -// An empty class with a constructor that's arranged invoked when all derived -// constructors of a mixin instance have completed and it is safe to allow GCs -// again. See AllocateObjectTrait<> comment for more. -// -// USING_GARBAGE_COLLECTED_MIXIN() declares a -// GarbageCollectedMixinConstructorMarker<> private field. By following Blink -// convention of using the macro at the top of a class declaration, its -// constructor will run first. -class GarbageCollectedMixinConstructorMarkerBase {}; -template <ThreadAffinity affinity> -class GarbageCollectedMixinConstructorMarker - : public GarbageCollectedMixinConstructorMarkerBase { - public: - GarbageCollectedMixinConstructorMarker() { - // FIXME: if prompt conservative GCs are needed, forced GCs that - // were denied while within this scope, could now be performed. - // For now, assume the next out-of-line allocation request will - // happen soon enough and take care of it. Mixin objects aren't - // overly common. - ThreadState* state = ThreadStateFor<affinity>::GetState(); - state->LeaveGCForbiddenScopeIfNeeded(this); - } -}; - // Merge two or more Mixins into one: // // class A : public GarbageCollectedMixin {}; // class B : public GarbageCollectedMixin {}; // class C : public A, public B { // // C::GetTraceDescriptor is now ambiguous because there are two -// candidates: -// // A::GetTraceDescriptor and B::GetTraceDescriptor. Ditto for other -// functions. +// // candidates: A::GetTraceDescriptor and B::GetTraceDescriptor. Ditto for +// // other functions. // // MERGE_GARBAGE_COLLECTED_MIXINS(); -// // The macro defines C::GetTraceDescriptor, etc. so that they are no -// longer -// // ambiguous. USING_GARBAGE_COLLECTED_MIXIN(TYPE) overrides them later -// // and provides the implementations. +// // The macro defines C::GetTraceDescriptor, similar to +// GarbageCollectedMixin, +// // so that they are no longer ambiguous. +// // USING_GARBAGE_COLLECTED_MIXIN(TYPE) overrides them later and provides +// // the implementations. // }; -#define MERGE_GARBAGE_COLLECTED_MIXINS() \ - public: \ - HeapObjectHeader* GetHeapObjectHeader() const override = 0; \ - TraceDescriptor GetTraceDescriptor() const override = 0; \ - \ - private: \ +#define MERGE_GARBAGE_COLLECTED_MIXINS() \ + public: \ + HeapObjectHeader* GetHeapObjectHeader() const override { \ + return reinterpret_cast<HeapObjectHeader*>( \ + BlinkGC::kNotFullyConstructedObject); \ + } \ + TraceDescriptor GetTraceDescriptor() const override { \ + return {BlinkGC::kNotFullyConstructedObject, nullptr, false}; \ + } \ + \ + private: \ using merge_garbage_collected_mixins_requires_semicolon = void // Base class for objects allocated in the Blink garbage-collected heap. diff --git a/chromium/third_party/blink/renderer/platform/heap/heap.cc b/chromium/third_party/blink/renderer/platform/heap/heap.cc index 8a1dc1bd0d8..cf507ab338a 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap.cc +++ b/chromium/third_party/blink/renderer/platform/heap/heap.cc @@ -142,29 +142,6 @@ Address ThreadHeap::CheckAndMarkPointer(MarkingVisitor* visitor, return nullptr; } -#if DCHECK_IS_ON() -// To support unit testing of the marking of off-heap root references -// into the heap, provide a checkAndMarkPointer() version with an -// extra notification argument. -Address ThreadHeap::CheckAndMarkPointer( - MarkingVisitor* visitor, - Address address, - MarkedPointerCallbackForTesting callback) { - DCHECK(thread_state_->InAtomicMarkingPause()); - - if (BasePage* page = LookupPageForAddress(address)) { - DCHECK(page->Contains(address)); - DCHECK(!address_cache_->Lookup(address)); - DCHECK(&visitor->Heap() == &page->Arena()->GetThreadState()->Heap()); - visitor->ConservativelyMarkAddress(page, address, callback); - return address; - } - if (!address_cache_->Lookup(address)) - address_cache_->AddEntry(address); - return nullptr; -} -#endif // DCHECK_IS_ON() - void ThreadHeap::RegisterWeakTable(void* table, EphemeronCallback iteration_callback) { DCHECK(thread_state_->InAtomicMarkingPause()); @@ -180,15 +157,39 @@ void ThreadHeap::RegisterWeakTable(void* table, void ThreadHeap::CommitCallbackStacks() { marking_worklist_.reset(new MarkingWorklist()); not_fully_constructed_worklist_.reset(new NotFullyConstructedWorklist()); + previously_not_fully_constructed_worklist_.reset( + new NotFullyConstructedWorklist()); weak_callback_worklist_.reset(new WeakCallbackWorklist()); DCHECK(ephemeron_callbacks_.IsEmpty()); } void ThreadHeap::DecommitCallbackStacks() { marking_worklist_.reset(nullptr); - not_fully_constructed_worklist_.reset(nullptr); + previously_not_fully_constructed_worklist_.reset(nullptr); weak_callback_worklist_.reset(nullptr); ephemeron_callbacks_.clear(); + + // The fixed point iteration may have found not-fully-constructed objects. + // Such objects should have already been found through the stack scan though + // and should thus already be marked. + if (!not_fully_constructed_worklist_->IsGlobalEmpty()) { +#if DCHECK_IS_ON() + NotFullyConstructedItem item; + while (not_fully_constructed_worklist_->Pop(WorklistTaskId::MainThread, + &item)) { + BasePage* const page = PageFromObject(item); + HeapObjectHeader* const header = + page->IsLargeObjectPage() + ? static_cast<LargeObjectPage*>(page)->ObjectHeader() + : static_cast<NormalPage*>(page)->FindHeaderFromAddress( + reinterpret_cast<Address>(const_cast<void*>(item))); + DCHECK(header->IsMarked()); + } +#else + not_fully_constructed_worklist_->Clear(); +#endif + } + not_fully_constructed_worklist_.reset(nullptr); } HeapCompact* ThreadHeap::Compaction() { @@ -207,6 +208,15 @@ void ThreadHeap::RegisterMovingObjectCallback(MovableReference* slot, Compaction()->RegisterMovingObjectCallback(slot, callback, callback_data); } +void ThreadHeap::FlushNotFullyConstructedObjects() { + if (!not_fully_constructed_worklist_->IsGlobalEmpty()) { + not_fully_constructed_worklist_->FlushToGlobal(WorklistTaskId::MainThread); + previously_not_fully_constructed_worklist_->MergeGlobalPool( + not_fully_constructed_worklist_.get()); + } + DCHECK(not_fully_constructed_worklist_->IsGlobalEmpty()); +} + void ThreadHeap::MarkNotFullyConstructedObjects(MarkingVisitor* visitor) { DCHECK(!thread_state_->IsIncrementalMarking()); ThreadHeapStatsCollector::Scope stats_scope( @@ -245,9 +255,33 @@ void ThreadHeap::InvokeEphemeronCallbacks(Visitor* visitor) { ephemeron_callbacks_ = std::move(final_set); } -bool ThreadHeap::AdvanceMarking(MarkingVisitor* visitor, TimeTicks deadline) { +namespace { + +template <typename Worklist, typename Callback> +bool DrainWorklistWithDeadline(TimeTicks deadline, + Worklist* worklist, + Callback callback) { const size_t kDeadlineCheckInterval = 2500; + size_t processed_callback_count = 0; + typename Worklist::EntryType item; + while (worklist->Pop(WorklistTaskId::MainThread, &item)) { + callback(item); + processed_callback_count++; + if (++processed_callback_count == kDeadlineCheckInterval) { + if (deadline <= CurrentTimeTicks()) { + return false; + } + processed_callback_count = 0; + } + } + return true; +} + +} // namespace + +bool ThreadHeap::AdvanceMarking(MarkingVisitor* visitor, TimeTicks deadline) { + bool finished; // Ephemeron fixed point loop. do { { @@ -255,16 +289,27 @@ bool ThreadHeap::AdvanceMarking(MarkingVisitor* visitor, TimeTicks deadline) { // currently pushed onto the marking worklist. ThreadHeapStatsCollector::Scope stats_scope( stats_collector(), ThreadHeapStatsCollector::kMarkProcessWorklist); - MarkingItem item; - while (marking_worklist_->Pop(WorklistTaskId::MainThread, &item)) { - item.callback(visitor, item.object); - processed_callback_count++; - if (processed_callback_count % kDeadlineCheckInterval == 0) { - if (deadline <= CurrentTimeTicks()) { - return false; - } - } - } + + finished = DrainWorklistWithDeadline( + deadline, marking_worklist_.get(), + [visitor](const MarkingItem& item) { + DCHECK(!HeapObjectHeader::FromPayload(item.object) + ->IsInConstruction()); + item.callback(visitor, item.object); + }); + if (!finished) + return false; + + // Iteratively mark all objects that were previously discovered while + // being in construction. The objects can be processed incrementally once + // a safepoint was reached. + finished = DrainWorklistWithDeadline( + deadline, previously_not_fully_constructed_worklist_.get(), + [visitor](const NotFullyConstructedItem& item) { + visitor->DynamicallyMarkAddress(reinterpret_cast<Address>(item)); + }); + if (!finished) + return false; } InvokeEphemeronCallbacks(visitor); @@ -576,6 +621,12 @@ void ThreadHeap::WriteBarrier(void* value) { if (header->IsMarked()) return; + if (header->IsInConstruction()) { + not_fully_constructed_worklist_->Push(WorklistTaskId::MainThread, + header->Payload()); + return; + } + // Mark and push trace callback. header->Mark(); marking_worklist_->Push( diff --git a/chromium/third_party/blink/renderer/platform/heap/heap.h b/chromium/third_party/blink/renderer/platform/heap/heap.h index 18d8498d8ea..0f81c2b2829 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap.h +++ b/chromium/third_party/blink/renderer/platform/heap/heap.h @@ -47,7 +47,6 @@ #include "third_party/blink/renderer/platform/wtf/address_sanitizer.h" #include "third_party/blink/renderer/platform/wtf/allocator.h" #include "third_party/blink/renderer/platform/wtf/assertions.h" -#include "third_party/blink/renderer/platform/wtf/atomics.h" #include "third_party/blink/renderer/platform/wtf/forward.h" namespace blink { @@ -122,6 +121,8 @@ class WeakMember; template <typename T> class UntracedMember; +namespace internal { + template <typename T, bool = NeedsAdjustPointer<T>::value> class ObjectAliveTrait; @@ -144,10 +145,17 @@ class ObjectAliveTrait<T, true> { NO_SANITIZE_ADDRESS static bool IsHeapObjectAlive(const T* object) { static_assert(sizeof(T), "T must be fully defined"); - return object->GetHeapObjectHeader()->IsMarked(); + const HeapObjectHeader* header = object->GetHeapObjectHeader(); + if (header == BlinkGC::kNotFullyConstructedObject) { + // Objects under construction are always alive. + return true; + } + return header->IsMarked(); } }; +} // namespace internal + class PLATFORM_EXPORT ThreadHeap { public: explicit ThreadHeap(ThreadState*); @@ -174,7 +182,7 @@ class PLATFORM_EXPORT ThreadHeap { return true; DCHECK(&ThreadState::Current()->Heap() == &PageFromObject(object)->Arena()->GetThreadState()->Heap()); - return ObjectAliveTrait<T>::IsHeapObjectAlive(object); + return internal::ObjectAliveTrait<T>::IsHeapObjectAlive(object); } template <typename T> static inline bool IsHeapObjectAlive(const Member<T>& member) { @@ -284,6 +292,11 @@ class PLATFORM_EXPORT ThreadHeap { void WeakProcessing(Visitor*); + // Moves not fully constructed objects to previously not fully constructed + // objects. Such objects can be iterated using the Trace() method and do + // not need to rely on conservative handling. + void FlushNotFullyConstructedObjects(); + // Marks not fully constructed objects. void MarkNotFullyConstructedObjects(MarkingVisitor*); // Marks the transitive closure including ephemerons. @@ -293,11 +306,6 @@ class PLATFORM_EXPORT ThreadHeap { // Conservatively checks whether an address is a pointer in any of the // thread heaps. If so marks the object pointed to as live. Address CheckAndMarkPointer(MarkingVisitor*, Address); -#if DCHECK_IS_ON() - Address CheckAndMarkPointer(MarkingVisitor*, - Address, - MarkedPointerCallbackForTesting); -#endif size_t ObjectPayloadSizeForTesting(); @@ -436,9 +444,29 @@ class PLATFORM_EXPORT ThreadHeap { std::unique_ptr<RegionTree> region_tree_; std::unique_ptr<AddressCache> address_cache_; std::unique_ptr<PagePool> free_page_pool_; + + // All objects on this worklist have been fully initialized and assigned a + // trace callback for iterating the body of the object. This worklist should + // contain almost all objects. std::unique_ptr<MarkingWorklist> marking_worklist_; + + // Objects on this worklist were observed to be in construction (in their + // constructor) and thus have been delayed for processing. They have not yet + // been assigned a valid header and trace callback. std::unique_ptr<NotFullyConstructedWorklist> not_fully_constructed_worklist_; + + // Objects on this worklist were previously in construction but have been + // moved here upon observing a safepoint, i.e., processing without stack. They + // have not yet been assigned a valid header and trace callback but are fully + // specified and can thus be iterated using the trace callback (which can be + // looked up dynamically). + std::unique_ptr<NotFullyConstructedWorklist> + previously_not_fully_constructed_worklist_; + + // Worklist of weak callbacks accumulated for objects. Such callbacks are + // processed after finishing marking objects. std::unique_ptr<WeakCallbackWorklist> weak_callback_worklist_; + // No duplicates allowed for ephemeron callbacks. Hence, we use a hashmap // with the key being the HashTable. WTF::HashMap<void*, EphemeronCallback> ephemeron_callbacks_; @@ -508,11 +536,14 @@ class GarbageCollected { public: using GarbageCollectedType = T; - void* operator new(size_t size) { - return AllocateObject(size, IsEagerlyFinalizedType<T>::value); - } + void* operator new(size_t size) = delete; // Must use MakeGarbageCollected. static void* AllocateObject(size_t size, bool eagerly_sweep) { + if (IsGarbageCollectedMixin<T>::value) { + // Ban large mixin so we can use PageFromObject() on them. + CHECK_GE(kLargeObjectSizeThreshold, size) + << "GarbageCollectedMixin may not be a large object"; + } return ThreadHeap::Allocate<T>(size, eagerly_sweep); } @@ -524,53 +555,18 @@ class GarbageCollected { DISALLOW_COPY_AND_ASSIGN(GarbageCollected); }; -template <typename T, bool is_mixin = IsGarbageCollectedMixin<T>::value> -class ConstructTrait { - public: -}; - -template <typename T> -class ConstructTrait<T, false> { - public: - template <typename... Args> - static T* Construct(Args&&... args) { - void* memory = - T::AllocateObject(sizeof(T), IsEagerlyFinalizedType<T>::value); - HeapObjectHeader* header = HeapObjectHeader::FromPayload(memory); - header->MarkIsInConstruction(); - // Placement new as regular operator new() is deleted. - T* object = ::new (memory) T(std::forward<Args>(args)...); - header->UnmarkIsInConstruction(); - return object; - } -}; - -template <typename T> -class ConstructTrait<T, true> { - public: - template <typename... Args> - NO_SANITIZE_UNRELATED_CAST static T* Construct(Args&&... args) { - void* memory = - T::AllocateObject(sizeof(T), IsEagerlyFinalizedType<T>::value); - HeapObjectHeader* header = HeapObjectHeader::FromPayload(memory); - header->MarkIsInConstruction(); - ThreadState* state = - ThreadStateFor<ThreadingTrait<T>::kAffinity>::GetState(); - state->EnterGCForbiddenScopeIfNeeded( - &(reinterpret_cast<T*>(memory)->mixin_constructor_marker_)); - // Placement new as regular operator new() is deleted. - T* object = ::new (memory) T(std::forward<Args>(args)...); - header->UnmarkIsInConstruction(); - return object; - } -}; - // Constructs an instance of T, which is a garbage collected type. template <typename T, typename... Args> T* MakeGarbageCollected(Args&&... args) { static_assert(WTF::IsGarbageCollectedType<T>::value, "T needs to be a garbage collected object"); - return ConstructTrait<T>::Construct(std::forward<Args>(args)...); + void* memory = T::AllocateObject(sizeof(T), IsEagerlyFinalizedType<T>::value); + HeapObjectHeader* header = HeapObjectHeader::FromPayload(memory); + header->MarkIsInConstruction(); + // Placement new as regular operator new() is deleted. + T* object = ::new (memory) T(std::forward<Args>(args)...); + header->UnmarkIsInConstruction(); + return object; } // Assigning class types to their arenas. @@ -731,7 +727,7 @@ void Visitor::HandleWeakCell(Visitor* self, void* object) { // preserved to avoid reviving objects in containers. return; } - if (!ObjectAliveTrait<T>::IsHeapObjectAlive(contents)) + if (!ThreadHeap::IsHeapObjectAlive(contents)) *cell = nullptr; } } diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_allocator.h b/chromium/third_party/blink/renderer/platform/heap/heap_allocator.h index fc47be71ac8..01d96002e48 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap_allocator.h +++ b/chromium/third_party/blink/renderer/platform/heap/heap_allocator.h @@ -6,6 +6,7 @@ #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_HEAP_ALLOCATOR_H_ #include "build/build_config.h" +#include "third_party/blink/renderer/platform/bindings/script_wrappable_marking_visitor.h" #include "third_party/blink/renderer/platform/heap/heap.h" #include "third_party/blink/renderer/platform/heap/heap_buildflags.h" #include "third_party/blink/renderer/platform/heap/marking_visitor.h" @@ -135,6 +136,17 @@ class PLATFORM_EXPORT HeapAllocator { MarkingVisitor::WriteBarrier(address); } + template <typename T> + static void BackingWriteBarrier(TraceWrapperMember<T>* address, size_t size) { + MarkingVisitor::WriteBarrier(address); + ScriptWrappableMarkingVisitor::WriteBarrier(address, size); + } + + template <typename T> + static void BackingWriteBarrier(T* address, size_t size) { + MarkingVisitor::WriteBarrier(address); + } + template <typename Return, typename Metadata> static Return Malloc(size_t size, const char* type_name) { return reinterpret_cast<Return>(ThreadHeap::Allocate<Metadata>( @@ -719,7 +731,8 @@ struct VectorTraits<blink::TraceWrapperMember<T>> static const bool kCanInitializeWithMemset = true; static const bool kCanClearUnusedSlotsWithMemset = true; static const bool kCanMoveWithMemcpy = true; - static const bool kCanSwapUsingCopyOrMove = false; + static const bool kCanCopyWithMemcpy = true; + static const bool kCanSwapUsingCopyOrMove = true; }; template <typename T> diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_compact.cc b/chromium/third_party/blink/renderer/platform/heap/heap_compact.cc index 551e0059f5a..055f2dc71ba 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap_compact.cc +++ b/chromium/third_party/blink/renderer/platform/heap/heap_compact.cc @@ -6,6 +6,7 @@ #include <memory> +#include "base/debug/alias.h" #include "base/memory/ptr_util.h" #include "third_party/blink/renderer/platform/heap/heap.h" #include "third_party/blink/renderer/platform/heap/heap_stats_collector.h" @@ -191,6 +192,16 @@ class HeapCompact::MovableObjectFixups final { DCHECK(relocatable_pages_.Contains(from_page)); #endif + // TODO(keishi): Code to determine if crash is related to interior fixups. + // Remove when finished. crbug.com/918064 + enum DebugSlotType { + kNormalSlot, + kInteriorSlotPreMove, + kInteriorSlotPostMove, + }; + DebugSlotType slot_type = kNormalSlot; + base::debug::Alias(&slot_type); + // If the object is referenced by a slot that is contained on a compacted // area itself, check whether it can be updated already. MovableReference* slot = reinterpret_cast<MovableReference*>(it->value); @@ -200,10 +211,12 @@ class HeapCompact::MovableObjectFixups final { reinterpret_cast<MovableReference*>(interior->value); if (!slot_location) { interior_fixups_.Set(slot, to); + slot_type = kInteriorSlotPreMove; } else { LOG_HEAP_COMPACTION() << "Redirected slot: " << slot << " => " << slot_location; slot = slot_location; + slot_type = kInteriorSlotPostMove; } } @@ -368,14 +381,10 @@ bool HeapCompact::ShouldCompact(ThreadHeap* heap, return force_compaction_gc_; } - // TODO(keishi): Should be enable after fixing the crashes. - if (marking_type == BlinkGC::kIncrementalMarking) - return false; - - // TODO(harukamt): Add kIncrementalIdleGC and kIncrementalV8FollowupGC when we - // enable heap compaction for incremental marking. if (reason != BlinkGC::GCReason::kIdleGC && reason != BlinkGC::GCReason::kPreciseGC && + reason != BlinkGC::GCReason::kIncrementalIdleGC && + reason != BlinkGC::GCReason::kIncrementalV8FollowupGC && reason != BlinkGC::GCReason::kForcedGC) return false; @@ -512,6 +521,16 @@ void HeapCompact::FinishThreadCompaction() { do_compact_ = false; } +void HeapCompact::CancelCompaction() { + if (!do_compact_) + return; + + last_fixup_count_for_testing_ = 0; + traced_slots_.clear(); + fixups_.reset(); + do_compact_ = false; +} + void HeapCompact::AddCompactingPage(BasePage* page) { DCHECK(do_compact_); DCHECK(IsCompactingArena(page->Arena()->ArenaIndex())); diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_compact.h b/chromium/third_party/blink/renderer/platform/heap/heap_compact.h index a41c5d39ef0..ec0ef448537 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap_compact.h +++ b/chromium/third_party/blink/renderer/platform/heap/heap_compact.h @@ -105,6 +105,8 @@ class PLATFORM_EXPORT HeapCompact final { void StartThreadCompaction(); void FinishThreadCompaction(); + void CancelCompaction(); + // Perform any relocation post-processing after having completed compacting // the given arena. The number of pages that were freed together with the // total size (in bytes) of freed heap storage, are passed in as arguments. diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_compact_test.cc b/chromium/third_party/blink/renderer/platform/heap/heap_compact_test.cc index c7035774eab..a09b609e77c 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap_compact_test.cc +++ b/chromium/third_party/blink/renderer/platform/heap/heap_compact_test.cc @@ -30,7 +30,7 @@ class IntWrapper : public blink::GarbageCollectedFinalized<IntWrapper> { static IntWrapper* Create(int x, VerifyArenaCompaction verify = NoVerify) { did_verify_at_least_once = false; - return new IntWrapper(x, verify); + return blink::MakeGarbageCollected<IntWrapper>(x, verify); } virtual ~IntWrapper() = default; diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_page.h b/chromium/third_party/blink/renderer/platform/heap/heap_page.h index b2625f01a6a..063a6e0a78b 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap_page.h +++ b/chromium/third_party/blink/renderer/platform/heap/heap_page.h @@ -584,7 +584,7 @@ class PLATFORM_EXPORT NormalPage final : public BasePage { // In order to use the same memory allocation routines for everything allocated // in the heap, large objects are considered heap pages containing only one // object. -class LargeObjectPage final : public BasePage { +class PLATFORM_EXPORT LargeObjectPage final : public BasePage { public: static size_t PageHeaderSize() { // Compute the amount of padding we have to add to a header to make the size diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_test.cc b/chromium/third_party/blink/renderer/platform/heap/heap_test.cc index d2f50322d53..64ab4fd551e 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap_test.cc +++ b/chromium/third_party/blink/renderer/platform/heap/heap_test.cc @@ -29,11 +29,14 @@ */ #include <algorithm> +#include <atomic> #include <memory> #include <utility> +#include "base/atomic_ref_count.h" #include "base/location.h" #include "base/memory/ptr_util.h" +#include "base/synchronization/waitable_event.h" #include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/public/platform/platform.h" @@ -61,11 +64,15 @@ namespace { class IntWrapper : public GarbageCollectedFinalized<IntWrapper> { public: - static IntWrapper* Create(int x) { return new IntWrapper(x); } + static IntWrapper* Create(int x) { + return MakeGarbageCollected<IntWrapper>(x); + } - virtual ~IntWrapper() { AtomicIncrement(&destructor_calls_); } + virtual ~IntWrapper() { + destructor_calls_.fetch_add(1, std::memory_order_relaxed); + } - static int destructor_calls_; + static std::atomic_int destructor_calls_; void Trace(blink::Visitor* visitor) {} int Value() const { return x_; } @@ -83,6 +90,8 @@ class IntWrapper : public GarbageCollectedFinalized<IntWrapper> { int x_; }; +std::atomic_int IntWrapper::destructor_calls_{0}; + struct IntWrapperHash { static unsigned GetHash(const IntWrapper& key) { return WTF::HashInt(static_cast<uint32_t>(key.Value())); @@ -381,7 +390,8 @@ class TestGCScope : public TestGCCollectGarbageScope { class SimpleObject : public GarbageCollected<SimpleObject> { public: - static SimpleObject* Create() { return new SimpleObject(); } + static SimpleObject* Create() { return MakeGarbageCollected<SimpleObject>(); } + SimpleObject() = default; void Trace(blink::Visitor* visitor) {} char GetPayload(int i) { return payload[i]; } // This virtual method is unused but it is here to make sure @@ -393,22 +403,21 @@ class SimpleObject : public GarbageCollected<SimpleObject> { virtual void VirtualMethod() {} protected: - SimpleObject() = default; char payload[64]; }; class HeapTestSuperClass : public GarbageCollectedFinalized<HeapTestSuperClass> { public: - static HeapTestSuperClass* Create() { return new HeapTestSuperClass(); } + static HeapTestSuperClass* Create() { + return MakeGarbageCollected<HeapTestSuperClass>(); + } + HeapTestSuperClass() = default; virtual ~HeapTestSuperClass() { ++destructor_calls_; } static int destructor_calls_; void Trace(blink::Visitor* visitor) {} - - protected: - HeapTestSuperClass() = default; }; int HeapTestSuperClass::destructor_calls_ = 0; @@ -423,8 +432,11 @@ static const size_t kClassMagic = 0xABCDDBCA; class HeapTestSubClass : public HeapTestOtherSuperClass, public HeapTestSuperClass { public: - static HeapTestSubClass* Create() { return new HeapTestSubClass(); } + static HeapTestSubClass* Create() { + return MakeGarbageCollected<HeapTestSubClass>(); + } + HeapTestSubClass() : magic_(kClassMagic) {} ~HeapTestSubClass() override { EXPECT_EQ(kClassMagic, magic_); ++destructor_calls_; @@ -433,8 +445,6 @@ class HeapTestSubClass : public HeapTestOtherSuperClass, static int destructor_calls_; private: - HeapTestSubClass() : magic_(kClassMagic) {} - const size_t magic_; }; @@ -483,24 +493,21 @@ class OffHeapInt : public RefCounted<OffHeapInt> { int x_; }; -int IntWrapper::destructor_calls_ = 0; int OffHeapInt::destructor_calls_ = 0; class ThreadedTesterBase { protected: static void Test(ThreadedTesterBase* tester) { - Vector<std::unique_ptr<Thread>, kNumberOfThreads> threads; - for (int i = 0; i < kNumberOfThreads; i++) { - threads.push_back(Platform::Current()->CreateThread( + std::unique_ptr<Thread> threads[kNumberOfThreads]; + for (auto& thread : threads) { + thread = Platform::Current()->CreateThread( ThreadCreationParams(WebThreadType::kTestThread) - .SetThreadNameForTest("blink gc testing thread"))); + .SetThreadNameForTest("blink gc testing thread")); PostCrossThreadTask( - *threads.back()->GetTaskRunner(), FROM_HERE, + *thread->GetTaskRunner(), FROM_HERE, CrossThreadBind(ThreadFunc, CrossThreadUnretained(tester))); } - while (AcquireLoad(&tester->threads_to_finish_)) { - test::YieldCurrentThread(); - } + tester->done_.Wait(); delete tester; } @@ -511,21 +518,26 @@ class ThreadedTesterBase { static const int kGcPerThread = 5; static const int kNumberOfAllocations = 50; - ThreadedTesterBase() : gc_count_(0), threads_to_finish_(kNumberOfThreads) {} - virtual ~ThreadedTesterBase() = default; inline bool Done() const { - return AcquireLoad(&gc_count_) >= kNumberOfThreads * kGcPerThread; + return gc_count_.load(std::memory_order_acquire) >= + kNumberOfThreads * kGcPerThread; } - volatile int gc_count_; - volatile int threads_to_finish_; + std::atomic_int gc_count_{0}; private: - static void ThreadFunc(void* data) { - reinterpret_cast<ThreadedTesterBase*>(data)->RunThread(); + static void ThreadFunc(ThreadedTesterBase* tester) { + ThreadState::AttachCurrentThread(); + tester->RunThread(); + ThreadState::DetachCurrentThread(); + if (!tester->threads_to_finish_.Decrement()) + tester->done_.Signal(); } + + base::AtomicRefCount threads_to_finish_{kNumberOfThreads}; + base::WaitableEvent done_; }; // Needed to give this variable a definition (the initializer above is only a @@ -563,8 +575,6 @@ class ThreadedHeapTester : public ThreadedTesterBase { } void RunThread() override { - ThreadState::AttachCurrentThread(); - // Add a cross-thread persistent from this thread; the test object // verifies that it will have been cleared out after the threads // have all detached, running their termination GCs while doing so. @@ -589,7 +599,7 @@ class ThreadedHeapTester : public ThreadedTesterBase { if (gc_count < kGcPerThread) { PreciselyCollectGarbage(); gc_count++; - AtomicIncrement(&gc_count_); + gc_count_.fetch_add(1, std::memory_order_release); } // Taking snapshot shouldn't have any bad side effect. @@ -603,9 +613,6 @@ class ThreadedHeapTester : public ThreadedTesterBase { } test::YieldCurrentThread(); } - - ThreadState::DetachCurrentThread(); - AtomicDecrement(&threads_to_finish_); } }; @@ -615,8 +622,6 @@ class ThreadedWeaknessTester : public ThreadedTesterBase { private: void RunThread() override { - ThreadState::AttachCurrentThread(); - int gc_count = 0; while (!Done()) { { @@ -632,7 +637,7 @@ class ThreadedWeaknessTester : public ThreadedTesterBase { if (gc_count < kGcPerThread) { PreciselyCollectGarbage(); gc_count++; - AtomicIncrement(&gc_count_); + gc_count_.fetch_add(1, std::memory_order_release); } // Taking snapshot shouldn't have any bad side effect. @@ -645,8 +650,6 @@ class ThreadedWeaknessTester : public ThreadedTesterBase { } test::YieldCurrentThread(); } - ThreadState::DetachCurrentThread(); - AtomicDecrement(&threads_to_finish_); } }; @@ -686,30 +689,26 @@ class ThreadPersistentHeapTester : public ThreadedTesterBase { class PersistentChain : public GarbageCollectedFinalized<PersistentChain> { public: static PersistentChain* Create(int count) { - return new PersistentChain(count); + return MakeGarbageCollected<PersistentChain>(count); } - void Trace(blink::Visitor* visitor) {} - - private: explicit PersistentChain(int count) { ref_counted_chain_ = base::AdoptRef(RefCountedChain::Create(count)); } + void Trace(blink::Visitor* visitor) {} + + private: scoped_refptr<RefCountedChain> ref_counted_chain_; }; void RunThread() override { - ThreadState::AttachCurrentThread(); - PersistentChain::Create(100); // Upon thread detach, GCs will run until all persistents have been // released. We verify that the draining of persistents proceeds // as expected by dropping one Persistent<> per GC until there // are none left. - ThreadState::DetachCurrentThread(); - AtomicDecrement(&threads_to_finish_); } }; @@ -724,14 +723,14 @@ void CheckWithSlack(T expected, T actual, int slack) { class TraceCounter : public GarbageCollectedFinalized<TraceCounter> { public: - static TraceCounter* Create() { return new TraceCounter(); } + static TraceCounter* Create() { return MakeGarbageCollected<TraceCounter>(); } + + TraceCounter() : trace_count_(0) {} void Trace(blink::Visitor* visitor) { trace_count_++; } int TraceCount() const { return trace_count_; } private: - TraceCounter() : trace_count_(0) {} - int trace_count_; }; @@ -747,7 +746,11 @@ TEST(HeapTest, IsHeapObjectAliveForConstPointer) { class ClassWithMember : public GarbageCollected<ClassWithMember> { public: - static ClassWithMember* Create() { return new ClassWithMember(); } + static ClassWithMember* Create() { + return MakeGarbageCollected<ClassWithMember>(); + } + + ClassWithMember() : trace_counter_(TraceCounter::Create()) {} void Trace(blink::Visitor* visitor) { visitor->Trace(trace_counter_); @@ -755,24 +758,22 @@ class ClassWithMember : public GarbageCollected<ClassWithMember> { int TraceCount() const { return trace_counter_->TraceCount(); } private: - ClassWithMember() : trace_counter_(TraceCounter::Create()) {} - Member<TraceCounter> trace_counter_; }; class SimpleFinalizedObject : public GarbageCollectedFinalized<SimpleFinalizedObject> { public: - static SimpleFinalizedObject* Create() { return new SimpleFinalizedObject(); } + static SimpleFinalizedObject* Create() { + return MakeGarbageCollected<SimpleFinalizedObject>(); + } + SimpleFinalizedObject() = default; ~SimpleFinalizedObject() { ++destructor_calls_; } static int destructor_calls_; void Trace(blink::Visitor* visitor) {} - - private: - SimpleFinalizedObject() = default; }; int SimpleFinalizedObject::destructor_calls_ = 0; @@ -809,7 +810,9 @@ class IntNode : public GarbageCollected<IntNode> { class Bar : public GarbageCollectedFinalized<Bar> { public: - static Bar* Create() { return new Bar(); } + static Bar* Create() { return MakeGarbageCollected<Bar>(); } + + Bar() : magic_(kMagic) { live_++; } void FinalizeGarbageCollectedObject() { EXPECT_TRUE(magic_ == kMagic); @@ -824,8 +827,6 @@ class Bar : public GarbageCollectedFinalized<Bar> { protected: static const int kMagic = 1337; int magic_; - - Bar() : magic_(kMagic) { live_++; } }; WILL_NOT_BE_EAGERLY_TRACED_CLASS(Bar); @@ -834,7 +835,9 @@ unsigned Bar::live_ = 0; class Baz : public GarbageCollected<Baz> { public: - static Baz* Create(Bar* bar) { return new Baz(bar); } + static Baz* Create(Bar* bar) { return MakeGarbageCollected<Baz>(bar); } + + explicit Baz(Bar* bar) : bar_(bar) {} void Trace(blink::Visitor* visitor) { visitor->Trace(bar_); } @@ -844,16 +847,18 @@ class Baz : public GarbageCollected<Baz> { void WillFinalize() { EXPECT_TRUE(!bar_->HasBeenFinalized()); } private: - explicit Baz(Bar* bar) : bar_(bar) {} - Member<Bar> bar_; }; class Foo : public Bar { public: - static Foo* Create(Bar* bar) { return new Foo(bar); } + static Foo* Create(Bar* bar) { return MakeGarbageCollected<Foo>(bar); } + + static Foo* Create(Foo* foo) { return MakeGarbageCollected<Foo>(foo); } + + Foo(Bar* bar) : Bar(), bar_(bar), points_to_foo_(false) {} - static Foo* Create(Foo* foo) { return new Foo(foo); } + Foo(Foo* foo) : Bar(), bar_(foo), points_to_foo_(true) {} void Trace(blink::Visitor* visitor) override { if (points_to_foo_) @@ -863,10 +868,6 @@ class Foo : public Bar { } private: - Foo(Bar* bar) : Bar(), bar_(bar), points_to_foo_(false) {} - - Foo(Foo* foo) : Bar(), bar_(foo), points_to_foo_(true) {} - Bar* bar_; bool points_to_foo_; }; @@ -875,7 +876,14 @@ WILL_NOT_BE_EAGERLY_TRACED_CLASS(Foo); class Bars : public Bar { public: - static Bars* Create() { return new Bars(); } + static Bars* Create() { return MakeGarbageCollected<Bars>(); } + + Bars() : width_(0) { + for (unsigned i = 0; i < kWidth; i++) { + bars_[i] = Bar::Create(); + width_++; + } + } void Trace(blink::Visitor* visitor) override { for (unsigned i = 0; i < width_; i++) @@ -887,13 +895,6 @@ class Bars : public Bar { static const unsigned kWidth = 7500; private: - Bars() : width_(0) { - for (unsigned i = 0; i < kWidth; i++) { - bars_[i] = Bar::Create(); - width_++; - } - } - unsigned width_; Member<Bar> bars_[kWidth]; }; @@ -902,20 +903,25 @@ WILL_NOT_BE_EAGERLY_TRACED_CLASS(Bars); class ConstructorAllocation : public GarbageCollected<ConstructorAllocation> { public: - static ConstructorAllocation* Create() { return new ConstructorAllocation(); } + static ConstructorAllocation* Create() { + return MakeGarbageCollected<ConstructorAllocation>(); + } + + ConstructorAllocation() { int_wrapper_ = IntWrapper::Create(42); } void Trace(blink::Visitor* visitor) { visitor->Trace(int_wrapper_); } private: - ConstructorAllocation() { int_wrapper_ = IntWrapper::Create(42); } - Member<IntWrapper> int_wrapper_; }; class LargeHeapObject : public GarbageCollectedFinalized<LargeHeapObject> { public: + LargeHeapObject() { int_wrapper_ = IntWrapper::Create(23); } ~LargeHeapObject() { destructor_calls_++; } - static LargeHeapObject* Create() { return new LargeHeapObject(); } + static LargeHeapObject* Create() { + return MakeGarbageCollected<LargeHeapObject>(); + } char Get(size_t i) { return data_[i]; } void Set(size_t i, char c) { data_[i] = c; } size_t length() { return kLength; } @@ -924,7 +930,6 @@ class LargeHeapObject : public GarbageCollectedFinalized<LargeHeapObject> { private: static const size_t kLength = 1024 * 1024; - LargeHeapObject() { int_wrapper_ = IntWrapper::Create(23); } Member<IntWrapper> int_wrapper_; char data_[kLength]; }; @@ -946,9 +951,10 @@ class RefCountedAndGarbageCollected : public GarbageCollectedFinalized<RefCountedAndGarbageCollected> { public: static RefCountedAndGarbageCollected* Create() { - return new RefCountedAndGarbageCollected; + return MakeGarbageCollected<RefCountedAndGarbageCollected>(); } + RefCountedAndGarbageCollected() : ref_count_(0) {} ~RefCountedAndGarbageCollected() { ++destructor_calls_; } void AddRef() { @@ -973,8 +979,6 @@ class RefCountedAndGarbageCollected static int destructor_calls_; private: - RefCountedAndGarbageCollected() : ref_count_(0) {} - int ref_count_; SelfKeepAlive<RefCountedAndGarbageCollected> keep_alive_; }; @@ -986,9 +990,10 @@ class RefCountedAndGarbageCollected2 public GarbageCollectedFinalized<RefCountedAndGarbageCollected2> { public: static RefCountedAndGarbageCollected2* Create() { - return new RefCountedAndGarbageCollected2; + return MakeGarbageCollected<RefCountedAndGarbageCollected2>(); } + RefCountedAndGarbageCollected2() : ref_count_(0) {} ~RefCountedAndGarbageCollected2() { ++destructor_calls_; } void Ref() { @@ -1013,8 +1018,6 @@ class RefCountedAndGarbageCollected2 static int destructor_calls_; private: - RefCountedAndGarbageCollected2() : ref_count_(0) {} - int ref_count_; SelfKeepAlive<RefCountedAndGarbageCollected2> keep_alive_; }; @@ -1023,7 +1026,12 @@ int RefCountedAndGarbageCollected2::destructor_calls_ = 0; class Weak : public Bar { public: - static Weak* Create(Bar* strong, Bar* weak) { return new Weak(strong, weak); } + static Weak* Create(Bar* strong, Bar* weak) { + return MakeGarbageCollected<Weak>(strong, weak); + } + + Weak(Bar* strong_bar, Bar* weak_bar) + : Bar(), strong_bar_(strong_bar), weak_bar_(weak_bar) {} void Trace(blink::Visitor* visitor) override { visitor->Trace(strong_bar_); @@ -1039,9 +1047,6 @@ class Weak : public Bar { bool WeakIsThere() { return !!weak_bar_; } private: - Weak(Bar* strong_bar, Bar* weak_bar) - : Bar(), strong_bar_(strong_bar), weak_bar_(weak_bar) {} - Member<Bar> strong_bar_; Bar* weak_bar_; }; @@ -1051,9 +1056,12 @@ WILL_NOT_BE_EAGERLY_TRACED_CLASS(Weak); class WithWeakMember : public Bar { public: static WithWeakMember* Create(Bar* strong, Bar* weak) { - return new WithWeakMember(strong, weak); + return MakeGarbageCollected<WithWeakMember>(strong, weak); } + WithWeakMember(Bar* strong_bar, Bar* weak_bar) + : Bar(), strong_bar_(strong_bar), weak_bar_(weak_bar) {} + void Trace(blink::Visitor* visitor) override { visitor->Trace(strong_bar_); visitor->Trace(weak_bar_); @@ -1063,9 +1071,6 @@ class WithWeakMember : public Bar { bool WeakIsThere() { return !!weak_bar_; } private: - WithWeakMember(Bar* strong_bar, Bar* weak_bar) - : Bar(), strong_bar_(strong_bar), weak_bar_(weak_bar) {} - Member<Bar> strong_bar_; WeakMember<Bar> weak_bar_; }; @@ -1076,7 +1081,10 @@ class Observable : public GarbageCollectedFinalized<Observable> { USING_PRE_FINALIZER(Observable, WillFinalize); public: - static Observable* Create(Bar* bar) { return new Observable(bar); } + static Observable* Create(Bar* bar) { + return MakeGarbageCollected<Observable>(bar); + } + explicit Observable(Bar* bar) : bar_(bar), was_destructed_(false) {} ~Observable() { was_destructed_ = true; } void Trace(blink::Visitor* visitor) { visitor->Trace(bar_); } @@ -1090,8 +1098,6 @@ class Observable : public GarbageCollectedFinalized<Observable> { static bool will_finalize_was_called_; private: - explicit Observable(Bar* bar) : bar_(bar), was_destructed_(false) {} - Member<Bar> bar_; bool was_destructed_; }; @@ -1104,8 +1110,9 @@ class ObservableWithPreFinalizer public: static ObservableWithPreFinalizer* Create() { - return new ObservableWithPreFinalizer(); + return MakeGarbageCollected<ObservableWithPreFinalizer>(); } + ObservableWithPreFinalizer() : was_destructed_(false) {} ~ObservableWithPreFinalizer() { was_destructed_ = true; } void Trace(blink::Visitor* visitor) {} void Dispose() { @@ -1115,8 +1122,6 @@ class ObservableWithPreFinalizer static bool dispose_was_called_; protected: - ObservableWithPreFinalizer() : was_destructed_(false) {} - bool was_destructed_; }; @@ -1130,7 +1135,10 @@ class PreFinalizerBase : public GarbageCollectedFinalized<PreFinalizerBase> { USING_PRE_FINALIZER(PreFinalizerBase, Dispose); public: - static PreFinalizerBase* Create() { return new PreFinalizerBase(); } + static PreFinalizerBase* Create() { + return MakeGarbageCollected<PreFinalizerBase>(); + } + PreFinalizerBase() : was_destructed_(false) {} virtual ~PreFinalizerBase() { was_destructed_ = true; } virtual void Trace(blink::Visitor* visitor) {} void Dispose() { @@ -1142,7 +1150,6 @@ class PreFinalizerBase : public GarbageCollectedFinalized<PreFinalizerBase> { } protected: - PreFinalizerBase() : was_destructed_(false) {} bool was_destructed_; }; @@ -1170,7 +1177,10 @@ class PreFinalizerSubClass : public PreFinalizerBase, public PreFinalizerMixin { USING_PRE_FINALIZER(PreFinalizerSubClass, Dispose); public: - static PreFinalizerSubClass* Create() { return new PreFinalizerSubClass(); } + static PreFinalizerSubClass* Create() { + return MakeGarbageCollected<PreFinalizerSubClass>(); + } + PreFinalizerSubClass() : was_destructed_(false) {} ~PreFinalizerSubClass() override { was_destructed_ = true; } void Trace(blink::Visitor* visitor) override {} void Dispose() { @@ -1182,7 +1192,6 @@ class PreFinalizerSubClass : public PreFinalizerBase, public PreFinalizerMixin { } protected: - PreFinalizerSubClass() : was_destructed_(false) {} bool was_destructed_; }; @@ -1190,8 +1199,11 @@ template <typename T> class FinalizationObserver : public GarbageCollected<FinalizationObserver<T>> { public: static FinalizationObserver* Create(T* data) { - return new FinalizationObserver(data); + return MakeGarbageCollected<FinalizationObserver>(data); } + + FinalizationObserver(T* data) : data_(data), did_call_will_finalize_(false) {} + bool DidCallWillFinalize() const { return did_call_will_finalize_; } void Trace(blink::Visitor* visitor) { @@ -1209,8 +1221,6 @@ class FinalizationObserver : public GarbageCollected<FinalizationObserver<T>> { } private: - FinalizationObserver(T* data) : data_(data), did_call_will_finalize_(false) {} - WeakMember<T> data_; bool did_call_will_finalize_; }; @@ -1268,8 +1278,9 @@ class SuperClass; class PointsBack : public GarbageCollectedFinalized<PointsBack> { public: - static PointsBack* Create() { return new PointsBack; } + static PointsBack* Create() { return MakeGarbageCollected<PointsBack>(); } + PointsBack() : back_pointer_(nullptr) { ++alive_count_; } ~PointsBack() { --alive_count_; } void SetBackPointer(SuperClass* back_pointer) { @@ -1283,8 +1294,6 @@ class PointsBack : public GarbageCollectedFinalized<PointsBack> { static int alive_count_; private: - PointsBack() : back_pointer_(nullptr) { ++alive_count_; } - WeakMember<SuperClass> back_pointer_; }; @@ -1293,9 +1302,13 @@ int PointsBack::alive_count_ = 0; class SuperClass : public GarbageCollectedFinalized<SuperClass> { public: static SuperClass* Create(PointsBack* points_back) { - return new SuperClass(points_back); + return MakeGarbageCollected<SuperClass>(points_back); } + explicit SuperClass(PointsBack* points_back) : points_back_(points_back) { + points_back_->SetBackPointer(this); + ++alive_count_; + } virtual ~SuperClass() { --alive_count_; } void DoStuff(SuperClass* target, @@ -1312,12 +1325,6 @@ class SuperClass : public GarbageCollectedFinalized<SuperClass> { static int alive_count_; - protected: - explicit SuperClass(PointsBack* points_back) : points_back_(points_back) { - points_back_->SetBackPointer(this); - ++alive_count_; - } - private: Member<PointsBack> points_back_; }; @@ -1338,9 +1345,13 @@ int SubData::alive_count_ = 0; class SubClass : public SuperClass { public: static SubClass* Create(PointsBack* points_back) { - return new SubClass(points_back); + return MakeGarbageCollected<SubClass>(points_back); } + explicit SubClass(PointsBack* points_back) + : SuperClass(points_back), data_(MakeGarbageCollected<SubData>()) { + ++alive_count_; + } ~SubClass() override { --alive_count_; } void Trace(blink::Visitor* visitor) override { @@ -1351,12 +1362,6 @@ class SubClass : public SuperClass { static int alive_count_; private: - explicit SubClass(PointsBack* points_back) - : SuperClass(points_back), data_(new SubData) { - ++alive_count_; - } - - private: Member<SubData> data_; }; @@ -1375,22 +1380,21 @@ class Mixin : public GarbageCollectedMixin { class UseMixin : public SimpleObject, public Mixin { USING_GARBAGE_COLLECTED_MIXIN(UseMixin) public: - static UseMixin* Create() { return new UseMixin(); } + static UseMixin* Create() { return MakeGarbageCollected<UseMixin>(); } - static int trace_count_; - void Trace(blink::Visitor* visitor) override { - SimpleObject::Trace(visitor); - Mixin::Trace(visitor); - ++trace_count_; - } - - private: UseMixin() { // Verify that WTF::IsGarbageCollectedType<> works as expected for mixins. static_assert(WTF::IsGarbageCollectedType<UseMixin>::value, "IsGarbageCollectedType<> sanity check failed for GC mixin."); trace_count_ = 0; } + + static int trace_count_; + void Trace(blink::Visitor* visitor) override { + SimpleObject::Trace(visitor); + Mixin::Trace(visitor); + ++trace_count_; + } }; int UseMixin::trace_count_ = 0; @@ -1488,7 +1492,7 @@ class FinalizationAllocator for (int i = 0; i < 10; ++i) *wrapper_ = IntWrapper::Create(42); for (int i = 0; i < 512; ++i) - new OneKiloByteObject(); + MakeGarbageCollected<OneKiloByteObject>(); for (int i = 0; i < 32; ++i) LargeHeapObject::Create(); } @@ -1511,7 +1515,7 @@ class PreFinalizationAllocator for (int i = 0; i < 10; ++i) *wrapper_ = IntWrapper::Create(42); for (int i = 0; i < 512; ++i) - new OneKiloByteObject(); + MakeGarbageCollected<OneKiloByteObject>(); for (int i = 0; i < 32; ++i) LargeHeapObject::Create(); } @@ -1529,12 +1533,12 @@ class PreFinalizerBackingShrinkForbidden public: PreFinalizerBackingShrinkForbidden() { for (int i = 0; i < 32; ++i) { - vector_.push_back(new IntWrapper(i)); + vector_.push_back(MakeGarbageCollected<IntWrapper>(i)); } EXPECT_LT(31ul, vector_.capacity()); for (int i = 0; i < 32; ++i) { - map_.insert(i + 1, new IntWrapper(i + 1)); + map_.insert(i + 1, MakeGarbageCollected<IntWrapper>(i + 1)); } EXPECT_LT(31ul, map_.Capacity()); } @@ -1572,7 +1576,7 @@ class PreFinalizerBackingShrinkForbidden }; TEST(HeapTest, PreFinalizerBackingShrinkForbidden) { - new PreFinalizerBackingShrinkForbidden(); + MakeGarbageCollected<PreFinalizerBackingShrinkForbidden>(); PreciselyCollectGarbage(); } @@ -1583,7 +1587,7 @@ class PreFinalizerVectorBackingExpandForbidden public: PreFinalizerVectorBackingExpandForbidden() { - vector_.push_back(new IntWrapper(1)); + vector_.push_back(MakeGarbageCollected<IntWrapper>(1)); } void Dispose() { EXPECT_DEATH(Test(), ""); } @@ -1602,7 +1606,7 @@ class PreFinalizerVectorBackingExpandForbidden }; TEST(HeapDeathTest, PreFinalizerVectorBackingExpandForbidden) { - new PreFinalizerVectorBackingExpandForbidden(); + MakeGarbageCollected<PreFinalizerVectorBackingExpandForbidden>(); PreciselyCollectGarbage(); } @@ -1613,7 +1617,7 @@ class PreFinalizerHashTableBackingExpandForbidden public: PreFinalizerHashTableBackingExpandForbidden() { - map_.insert(123, new IntWrapper(123)); + map_.insert(123, MakeGarbageCollected<IntWrapper>(123)); } void Dispose() { EXPECT_DEATH(Test(), ""); } @@ -1632,7 +1636,7 @@ class PreFinalizerHashTableBackingExpandForbidden }; TEST(HeapDeathTest, PreFinalizerHashTableBackingExpandForbidden) { - new PreFinalizerHashTableBackingExpandForbidden(); + MakeGarbageCollected<PreFinalizerHashTableBackingExpandForbidden>(); PreciselyCollectGarbage(); } @@ -1644,7 +1648,7 @@ class LargeMixin : public GarbageCollected<LargeMixin>, public Mixin { }; TEST(HeapDeathTest, LargeGarbageCollectedMixin) { - EXPECT_DEATH(new LargeMixin(), ""); + EXPECT_DEATH(MakeGarbageCollected<LargeMixin>(), ""); } TEST(HeapTest, Transition) { @@ -1874,7 +1878,7 @@ TEST(HeapTest, SimpleAllocation) { EXPECT_EQ(0ul, heap.ObjectPayloadSizeForTesting()); // Allocate an object in the heap. - HeapAllocatedArray* array = new HeapAllocatedArray(); + HeapAllocatedArray* array = MakeGarbageCollected<HeapAllocatedArray>(); EXPECT_TRUE(heap.ObjectPayloadSizeForTesting() >= sizeof(HeapAllocatedArray)); // Sanity check of the contents in the heap. @@ -1918,13 +1922,13 @@ TEST(HeapTest, FreelistReuse) { ClearOutOldGarbage(); for (int i = 0; i < 100; i++) - new IntWrapper(i); - IntWrapper* p1 = new IntWrapper(100); + MakeGarbageCollected<IntWrapper>(i); + IntWrapper* p1 = MakeGarbageCollected<IntWrapper>(100); PreciselyCollectGarbage(); // In non-production builds, we delay reusing freed memory for at least // one GC cycle. for (int i = 0; i < 100; i++) { - IntWrapper* p2 = new IntWrapper(i); + IntWrapper* p2 = MakeGarbageCollected<IntWrapper>(i); EXPECT_NE(p1, p2); } @@ -1933,7 +1937,7 @@ TEST(HeapTest, FreelistReuse) { // Now the freed memory in the first GC should be reused. bool reused_memory_found = false; for (int i = 0; i < 10000; i++) { - IntWrapper* p2 = new IntWrapper(i); + IntWrapper* p2 = MakeGarbageCollected<IntWrapper>(i); if (p1 == p2) { reused_memory_found = true; break; @@ -1966,11 +1970,11 @@ TEST(HeapTest, LazySweepingLargeObjectPages) { // Create free lists that can be reused for IntWrappers created in // LargeHeapObject::create(). - Persistent<IntWrapper> p1 = new IntWrapper(1); + Persistent<IntWrapper> p1 = MakeGarbageCollected<IntWrapper>(1); for (int i = 0; i < 100; i++) { - new IntWrapper(i); + MakeGarbageCollected<IntWrapper>(i); } - Persistent<IntWrapper> p2 = new IntWrapper(2); + Persistent<IntWrapper> p2 = MakeGarbageCollected<IntWrapper>(2); PreciselyCollectGarbage(); PreciselyCollectGarbage(); @@ -2012,15 +2016,13 @@ class SimpleFinalizedEagerObjectBase class SimpleFinalizedEagerObject : public SimpleFinalizedEagerObjectBase { public: static SimpleFinalizedEagerObject* Create() { - return new SimpleFinalizedEagerObject(); + return MakeGarbageCollected<SimpleFinalizedEagerObject>(); } + SimpleFinalizedEagerObject() = default; ~SimpleFinalizedEagerObject() override { ++destructor_calls_; } static int destructor_calls_; - - private: - SimpleFinalizedEagerObject() = default; }; template <typename T> @@ -2034,16 +2036,15 @@ class SimpleFinalizedObjectInstanceOfTemplate final public ParameterizedButEmpty<SimpleFinalizedObjectInstanceOfTemplate> { public: static SimpleFinalizedObjectInstanceOfTemplate* Create() { - return new SimpleFinalizedObjectInstanceOfTemplate(); + return MakeGarbageCollected<SimpleFinalizedObjectInstanceOfTemplate>(); } + + SimpleFinalizedObjectInstanceOfTemplate() = default; ~SimpleFinalizedObjectInstanceOfTemplate() { ++destructor_calls_; } void Trace(blink::Visitor* visitor) {} static int destructor_calls_; - - private: - SimpleFinalizedObjectInstanceOfTemplate() = default; }; int SimpleFinalizedEagerObject::destructor_calls_ = 0; @@ -2423,7 +2424,7 @@ typedef std::pair<int, WeakMember<IntWrapper>> PairUnwrappedWeak; class Container : public GarbageCollected<Container> { public: - static Container* Create() { return new Container(); } + static Container* Create() { return MakeGarbageCollected<Container>(); } HeapHashMap<Member<IntWrapper>, Member<IntWrapper>> map; HeapHashSet<Member<IntWrapper>> set; HeapHashSet<Member<IntWrapper>> set2; @@ -3848,9 +3849,9 @@ TEST(HeapTest, HeapWeakCollectionTypes) { TEST(HeapTest, HeapHashCountedSetToVector) { HeapHashCountedSet<Member<IntWrapper>> set; HeapVector<Member<IntWrapper>> vector; - set.insert(new IntWrapper(1)); - set.insert(new IntWrapper(1)); - set.insert(new IntWrapper(2)); + set.insert(MakeGarbageCollected<IntWrapper>(1)); + set.insert(MakeGarbageCollected<IntWrapper>(1)); + set.insert(MakeGarbageCollected<IntWrapper>(2)); CopyToVector(set, vector); EXPECT_EQ(3u, vector.size()); @@ -3868,9 +3869,9 @@ TEST(HeapTest, HeapHashCountedSetToVector) { TEST(HeapTest, WeakHeapHashCountedSetToVector) { HeapHashCountedSet<WeakMember<IntWrapper>> set; HeapVector<Member<IntWrapper>> vector; - set.insert(new IntWrapper(1)); - set.insert(new IntWrapper(1)); - set.insert(new IntWrapper(2)); + set.insert(MakeGarbageCollected<IntWrapper>(1)); + set.insert(MakeGarbageCollected<IntWrapper>(1)); + set.insert(MakeGarbageCollected<IntWrapper>(2)); CopyToVector(set, vector); EXPECT_LE(3u, vector.size()); @@ -4014,104 +4015,96 @@ TEST(HeapTest, Comparisons) { EXPECT_TRUE(bar_persistent == foo_persistent); } -#if DCHECK_IS_ON() namespace { -static size_t g_check_mark_count = 0; - -bool ReportMarkedPointer(HeapObjectHeader*) { - g_check_mark_count++; - // Do not try to mark the located heap object. - return true; +void ExpectObjectMarkedAndUnmark(MarkingWorklist* worklist, void* expected) { + MarkingItem item; + CHECK(worklist->Pop(0, &item)); + CHECK_EQ(expected, item.object); + HeapObjectHeader* header = HeapObjectHeader::FromPayload(item.object); + CHECK(header->IsMarked()); + header->Unmark(); + CHECK(worklist->IsGlobalEmpty()); } -} -#endif + +} // namespace TEST(HeapTest, CheckAndMarkPointer) { -#if DCHECK_IS_ON() + // This test ensures that conservative marking primitives can use any address + // contained within an object to mark the corresponding object. + ThreadHeap& heap = ThreadState::Current()->Heap(); ClearOutOldGarbage(); Vector<Address> object_addresses; Vector<Address> end_addresses; - Address large_object_address; - Address large_object_end_address; for (int i = 0; i < 10; i++) { SimpleObject* object = SimpleObject::Create(); Address object_address = reinterpret_cast<Address>(object); object_addresses.push_back(object_address); end_addresses.push_back(object_address + sizeof(SimpleObject) - 1); } - LargeHeapObject* large_object = LargeHeapObject::Create(); - large_object_address = reinterpret_cast<Address>(large_object); - large_object_end_address = large_object_address + sizeof(LargeHeapObject) - 1; - - // This is a low-level test where we call checkAndMarkPointer. This method - // causes the object start bitmap to be computed which requires the heap - // to be in a consistent state (e.g. the free allocation area must be put - // into a free list header). However when we call makeConsistentForGC it - // also clears out the freelists so we have to rebuild those before trying - // to allocate anything again. We do this by forcing a GC after doing the - // checkAndMarkPointer tests. + Address large_object_address = + reinterpret_cast<Address>(LargeHeapObject::Create()); + Address large_object_end_address = + large_object_address + sizeof(LargeHeapObject) - 1; + { TestGCScope scope(BlinkGC::kHeapPointersOnStack); MarkingVisitor visitor(ThreadState::Current(), MarkingVisitor::kGlobalMarking); heap.address_cache()->EnableLookup(); heap.address_cache()->Flush(); + + // Conservative marker should find the interesting objects by using anything + // between object start and end. + MarkingWorklist* worklist = heap.GetMarkingWorklist(); + CHECK(worklist->IsGlobalEmpty()); for (wtf_size_t i = 0; i < object_addresses.size(); i++) { - EXPECT_TRUE(heap.CheckAndMarkPointer(&visitor, object_addresses[i], - ReportMarkedPointer)); - EXPECT_TRUE(heap.CheckAndMarkPointer(&visitor, end_addresses[i], - ReportMarkedPointer)); + heap.CheckAndMarkPointer(&visitor, object_addresses[i]); + ExpectObjectMarkedAndUnmark(worklist, object_addresses[i]); + heap.CheckAndMarkPointer(&visitor, end_addresses[i]); + ExpectObjectMarkedAndUnmark(worklist, object_addresses[i]); } - EXPECT_EQ(object_addresses.size() * 2, g_check_mark_count); - g_check_mark_count = 0; - EXPECT_TRUE(heap.CheckAndMarkPointer(&visitor, large_object_address, - ReportMarkedPointer)); - EXPECT_TRUE(heap.CheckAndMarkPointer(&visitor, large_object_end_address, - ReportMarkedPointer)); - EXPECT_EQ(2ul, g_check_mark_count); - g_check_mark_count = 0ul; + heap.CheckAndMarkPointer(&visitor, large_object_address); + ExpectObjectMarkedAndUnmark(worklist, large_object_address); + heap.CheckAndMarkPointer(&visitor, large_object_end_address); + ExpectObjectMarkedAndUnmark(worklist, large_object_address); } + // This forces a GC without stack scanning which results in the objects // being collected. This will also rebuild the above mentioned freelists, // however we don't rely on that below since we don't have any allocations. ClearOutOldGarbage(); + { TestGCScope scope(BlinkGC::kHeapPointersOnStack); MarkingVisitor visitor(ThreadState::Current(), MarkingVisitor::kGlobalMarking); heap.address_cache()->EnableLookup(); heap.address_cache()->Flush(); + + // After collecting all interesting objects the conservative marker should + // not find them anymore. + MarkingWorklist* worklist = heap.GetMarkingWorklist(); + CHECK(worklist->IsGlobalEmpty()); for (wtf_size_t i = 0; i < object_addresses.size(); i++) { - // We would like to assert that checkAndMarkPointer returned false - // here because the pointers no longer point into a valid object - // (it's been freed by the GCs. But checkAndMarkPointer will return - // true for any pointer that points into a heap page, regardless of - // whether it points at a valid object (this ensures the - // correctness of the page-based on-heap address caches), so we - // can't make that assert. - heap.CheckAndMarkPointer(&visitor, object_addresses[i], - ReportMarkedPointer); - heap.CheckAndMarkPointer(&visitor, end_addresses[i], ReportMarkedPointer); + heap.CheckAndMarkPointer(&visitor, object_addresses[i]); + CHECK(worklist->IsGlobalEmpty()); + heap.CheckAndMarkPointer(&visitor, end_addresses[i]); + CHECK(worklist->IsGlobalEmpty()); } - EXPECT_EQ(0ul, g_check_mark_count); - heap.CheckAndMarkPointer(&visitor, large_object_address, - ReportMarkedPointer); - heap.CheckAndMarkPointer(&visitor, large_object_end_address, - ReportMarkedPointer); - EXPECT_EQ(0ul, g_check_mark_count); - } - // This round of GC is important to make sure that the object start - // bitmap are cleared out and that the free lists are rebuild. - ClearOutOldGarbage(); -#endif + heap.CheckAndMarkPointer(&visitor, large_object_address); + CHECK(worklist->IsGlobalEmpty()); + heap.CheckAndMarkPointer(&visitor, large_object_end_address); + CHECK(worklist->IsGlobalEmpty()); + } } TEST(HeapTest, CollectionNesting) { ClearOutOldGarbage(); - int* key = &IntWrapper::destructor_calls_; + int k; + int* key = &k; IntWrapper::destructor_calls_ = 0; typedef HeapVector<Member<IntWrapper>> IntVector; typedef HeapDeque<Member<IntWrapper>> IntDeque; @@ -4409,7 +4402,7 @@ TEST(HeapTest, VectorDestructors) { InlinedVectorObject::destructor_calls_ = 0; { Persistent<InlinedVectorObjectWrapper> vector_wrapper = - new InlinedVectorObjectWrapper(); + MakeGarbageCollected<InlinedVectorObjectWrapper>(); ConservativelyCollectGarbage(); EXPECT_EQ(2, InlinedVectorObject::destructor_calls_); } @@ -4455,7 +4448,7 @@ TEST(HeapTest, VectorDestructorsWithVtable) { InlinedVectorObjectWithVtable::destructor_calls_ = 0; { Persistent<InlinedVectorObjectWithVtableWrapper> vector_wrapper = - new InlinedVectorObjectWithVtableWrapper(); + MakeGarbageCollected<InlinedVectorObjectWithVtableWrapper>(); ConservativelyCollectGarbage(); EXPECT_EQ(3, InlinedVectorObjectWithVtable::destructor_calls_); } @@ -4481,7 +4474,7 @@ TEST(HeapTest, HeapLinkedStack) { IntWrapper::destructor_calls_ = 0; HeapLinkedStack<TerminatedArrayItem>* stack = - new HeapLinkedStack<TerminatedArrayItem>(); + MakeGarbageCollected<HeapLinkedStack<TerminatedArrayItem>>(); const wtf_size_t kStackSize = 10; @@ -4511,7 +4504,7 @@ TEST(HeapTest, AllocationDuringFinalization) { LargeHeapObject::destructor_calls_ = 0; Persistent<IntWrapper> wrapper; - new FinalizationAllocator(&wrapper); + MakeGarbageCollected<FinalizationAllocator>(&wrapper); PreciselyCollectGarbage(); EXPECT_EQ(0, IntWrapper::destructor_calls_); @@ -4537,7 +4530,7 @@ TEST(HeapTest, AllocationDuringPrefinalizer) { LargeHeapObject::destructor_calls_ = 0; Persistent<IntWrapper> wrapper; - new PreFinalizationAllocator(&wrapper); + MakeGarbageCollected<PreFinalizationAllocator>(&wrapper); PreciselyCollectGarbage(); EXPECT_EQ(0, IntWrapper::destructor_calls_); @@ -4694,7 +4687,7 @@ TEST(HeapTest, MultipleMixins) { ClearOutOldGarbage(); IntWrapper::destructor_calls_ = 0; - MultipleMixins* obj = new MultipleMixins(); + MultipleMixins* obj = MakeGarbageCollected<MultipleMixins>(); { Persistent<MixinA> a = obj; PreciselyCollectGarbage(); @@ -4714,7 +4707,7 @@ TEST(HeapTest, DerivedMultipleMixins) { IntWrapper::destructor_calls_ = 0; DerivedMultipleMixins::trace_called_ = 0; - DerivedMultipleMixins* obj = new DerivedMultipleMixins(); + DerivedMultipleMixins* obj = MakeGarbageCollected<DerivedMultipleMixins>(); { Persistent<MixinA> a = obj; PreciselyCollectGarbage(); @@ -4745,7 +4738,8 @@ TEST(HeapTest, MixinInstanceWithoutTrace) { // references inherits the mixin's trace implementation. ClearOutOldGarbage(); MixinA::trace_count_ = 0; - MixinInstanceWithoutTrace* obj = new MixinInstanceWithoutTrace(); + MixinInstanceWithoutTrace* obj = + MakeGarbageCollected<MixinInstanceWithoutTrace>(); int saved_trace_count = 0; { Persistent<MixinA> a = obj; @@ -5216,12 +5210,13 @@ TEST(HeapTest, EphemeronsPointToEphemerons) { Persistent<EphemeronWrapper> chain; for (int i = 0; i < 100; i++) { EphemeronWrapper* old_head = chain; - chain = new EphemeronWrapper(); + chain = MakeGarbageCollected<EphemeronWrapper>(); if (i == 50) chain->GetMap().insert(key2, old_head); else chain->GetMap().insert(key, old_head); - chain->GetMap().insert(IntWrapper::Create(103), new EphemeronWrapper()); + chain->GetMap().insert(IntWrapper::Create(103), + MakeGarbageCollected<EphemeronWrapper>()); } PreciselyCollectGarbage(); @@ -5351,8 +5346,8 @@ TEST(HeapTest, IndirectStrongToWeak) { Persistent<IntWrapper> dead_object = IntWrapper::Create(100); // Named for "Drowning by Numbers" (1988). Persistent<IntWrapper> life_object = IntWrapper::Create(42); - map->insert(dead_object, new Link1(dead_object)); - map->insert(life_object, new Link1(life_object)); + map->insert(dead_object, MakeGarbageCollected<Link1>(dead_object)); + map->insert(life_object, MakeGarbageCollected<Link1>(life_object)); EXPECT_EQ(2u, map->size()); PreciselyCollectGarbage(); EXPECT_EQ(2u, map->size()); @@ -5367,370 +5362,11 @@ TEST(HeapTest, IndirectStrongToWeak) { EXPECT_EQ(0u, map->size()); } -static Mutex& MainThreadMutex() { - DEFINE_THREAD_SAFE_STATIC_LOCAL(Mutex, main_mutex, ()); - return main_mutex; -} - -static ThreadCondition& MainThreadCondition() { - DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadCondition, main_condition, - (MainThreadMutex())); - return main_condition; -} - -static void ParkMainThread() { - MainThreadCondition().Wait(); -} - -static void WakeMainThread() { - MutexLocker locker(MainThreadMutex()); - MainThreadCondition().Signal(); -} - -static Mutex& WorkerThreadMutex() { - DEFINE_THREAD_SAFE_STATIC_LOCAL(Mutex, worker_mutex, ()); - return worker_mutex; -} - -static ThreadCondition& WorkerThreadCondition() { - DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadCondition, worker_condition, - (WorkerThreadMutex())); - return worker_condition; -} - -static void ParkWorkerThread() { - WorkerThreadCondition().Wait(); -} - -static void WakeWorkerThread() { - MutexLocker locker(WorkerThreadMutex()); - WorkerThreadCondition().Signal(); -} - -class ThreadedStrongificationTester { - public: - static void Test() { - IntWrapper::destructor_calls_ = 0; - - MutexLocker locker(MainThreadMutex()); - std::unique_ptr<Thread> worker_thread = Platform::Current()->CreateThread( - ThreadCreationParams(WebThreadType::kTestThread) - .SetThreadNameForTest("Test Worker Thread")); - PostCrossThreadTask(*worker_thread->GetTaskRunner(), FROM_HERE, - CrossThreadBind(WorkerThreadMain)); - - // Wait for the worker thread initialization. The worker - // allocates a weak collection where both collection and - // contents are kept alive via persistent pointers. - ParkMainThread(); - - // Perform two garbage collections where the worker thread does - // not wake up in between. This will cause us to remove marks - // and mark unmarked objects dead. The collection on the worker - // heap is found through the persistent and the backing should - // be marked. - PreciselyCollectGarbage(); - PreciselyCollectGarbage(); - - // Wake up the worker thread so it can continue. It will sweep - // and perform another GC where the backing store of its - // collection should be strongified. - WakeWorkerThread(); - - // Wait for the worker thread to sweep its heaps before checking. - ParkMainThread(); - } - - private: - using WeakCollectionType = - HeapHashMap<WeakMember<IntWrapper>, Member<IntWrapper>>; - - static WeakCollectionType* AllocateCollection() { - // Create a weak collection that is kept alive by a persistent - // and keep the contents alive with a persistents as - // well. - Persistent<IntWrapper> wrapper1 = IntWrapper::Create(32); - Persistent<IntWrapper> wrapper2 = IntWrapper::Create(32); - Persistent<IntWrapper> wrapper3 = IntWrapper::Create(32); - Persistent<IntWrapper> wrapper4 = IntWrapper::Create(32); - Persistent<IntWrapper> wrapper5 = IntWrapper::Create(32); - Persistent<IntWrapper> wrapper6 = IntWrapper::Create(32); - Persistent<WeakCollectionType> weak_collection = - MakeGarbageCollected<WeakCollectionType>(); - weak_collection->insert(wrapper1, wrapper1); - weak_collection->insert(wrapper2, wrapper2); - weak_collection->insert(wrapper3, wrapper3); - weak_collection->insert(wrapper4, wrapper4); - weak_collection->insert(wrapper5, wrapper5); - weak_collection->insert(wrapper6, wrapper6); - - // Signal the main thread that the worker is done with its allocation. - WakeMainThread(); - - // Wait for the main thread to do two GCs without sweeping - // this thread heap. - ParkWorkerThread(); - - return weak_collection; - } - - static void WorkerThreadMain() { - MutexLocker locker(WorkerThreadMutex()); - - ThreadState::AttachCurrentThread(); - - { - Persistent<WeakCollectionType> collection = AllocateCollection(); - { - // Prevent weak processing with an iterator and GC. - WeakCollectionType::iterator it = collection->begin(); - ConservativelyCollectGarbage(); - - // The backing should be strongified because of the iterator. - EXPECT_EQ(6u, collection->size()); - EXPECT_EQ(32, it->value->Value()); - } - - // Disregarding the iterator but keeping the collection alive - // with a persistent should lead to weak processing. - PreciselyCollectGarbage(); - EXPECT_EQ(0u, collection->size()); - } - - WakeMainThread(); - ThreadState::DetachCurrentThread(); - } - - static volatile uintptr_t worker_object_pointer_; -}; - -TEST(HeapTest, ThreadedStrongification) { - ThreadedStrongificationTester::Test(); -} - -class MemberSameThreadCheckTester { - public: - void Test() { - IntWrapper::destructor_calls_ = 0; - - MutexLocker locker(MainThreadMutex()); - std::unique_ptr<Thread> worker_thread = Platform::Current()->CreateThread( - ThreadCreationParams(WebThreadType::kTestThread) - .SetThreadNameForTest("Test Worker Thread")); - PostCrossThreadTask( - *worker_thread->GetTaskRunner(), FROM_HERE, - CrossThreadBind(&MemberSameThreadCheckTester::WorkerThreadMain, - CrossThreadUnretained(this))); - - ParkMainThread(); - } - - private: - Member<IntWrapper> wrapper_; - - void WorkerThreadMain() { - MutexLocker locker(WorkerThreadMutex()); - - ThreadState::AttachCurrentThread(); - - // Setting an object created on the worker thread to a Member allocated on - // the main thread is not allowed. - wrapper_ = IntWrapper::Create(42); - - WakeMainThread(); - ThreadState::DetachCurrentThread(); - } -}; - -#if DCHECK_IS_ON() -// TODO(keishi) This test is flaky on mac_chromium_rel_ng bot. -// crbug.com/709069 -#if !defined(OS_MACOSX) -TEST(HeapDeathTest, MemberSameThreadCheck) { - EXPECT_DEATH(MemberSameThreadCheckTester().Test(), ""); -} -#endif -#endif - -class PersistentSameThreadCheckTester { - public: - void Test() { - IntWrapper::destructor_calls_ = 0; - - MutexLocker locker(MainThreadMutex()); - std::unique_ptr<Thread> worker_thread = Platform::Current()->CreateThread( - ThreadCreationParams(WebThreadType::kTestThread) - .SetThreadNameForTest("Test Worker Thread")); - PostCrossThreadTask( - *worker_thread->GetTaskRunner(), FROM_HERE, - CrossThreadBind(&PersistentSameThreadCheckTester::WorkerThreadMain, - CrossThreadUnretained(this))); - - ParkMainThread(); - } - - private: - Persistent<IntWrapper> wrapper_; - - void WorkerThreadMain() { - MutexLocker locker(WorkerThreadMutex()); - - ThreadState::AttachCurrentThread(); - - // Setting an object created on the worker thread to a Persistent allocated - // on the main thread is not allowed. - wrapper_ = IntWrapper::Create(42); - - WakeMainThread(); - ThreadState::DetachCurrentThread(); - } -}; - -#if DCHECK_IS_ON() -// TODO(keishi) This test is flaky on mac_chromium_rel_ng bot. -// crbug.com/709069 -#if !defined(OS_MACOSX) -TEST(HeapDeathTest, PersistentSameThreadCheck) { - EXPECT_DEATH(PersistentSameThreadCheckTester().Test(), ""); -} -#endif -#endif - -class MarkingSameThreadCheckTester { - public: - void Test() { - IntWrapper::destructor_calls_ = 0; - - MutexLocker locker(MainThreadMutex()); - std::unique_ptr<Thread> worker_thread = Platform::Current()->CreateThread( - ThreadCreationParams(WebThreadType::kTestThread) - .SetThreadNameForTest("Test Worker Thread")); - Persistent<MainThreadObject> main_thread_object = new MainThreadObject(); - PostCrossThreadTask( - *worker_thread->GetTaskRunner(), FROM_HERE, - CrossThreadBind(&MarkingSameThreadCheckTester::WorkerThreadMain, - CrossThreadUnretained(this), - WrapCrossThreadPersistent(main_thread_object.Get()))); - ParkMainThread(); - // This will try to mark MainThreadObject when it tries to mark IntWrapper - // it should crash. - PreciselyCollectGarbage(); - } - - private: - class MainThreadObject : public GarbageCollectedFinalized<MainThreadObject> { - public: - void Trace(blink::Visitor* visitor) { visitor->Trace(wrapper_set_); } - void AddToSet(IntWrapper* wrapper) { wrapper_set_.insert(42, wrapper); } - - private: - HeapHashMap<int, Member<IntWrapper>> wrapper_set_; - }; - - void WorkerThreadMain(MainThreadObject* main_thread_object) { - MutexLocker locker(WorkerThreadMutex()); - - ThreadState::AttachCurrentThread(); - - // Adding a reference to an object created on the worker thread to a - // HeapHashMap created on the main thread is not allowed. - main_thread_object->AddToSet(IntWrapper::Create(42)); - - WakeMainThread(); - ThreadState::DetachCurrentThread(); - } -}; - -#if DCHECK_IS_ON() -// TODO(keishi) This test is flaky on mac_chromium_rel_ng bot. -// crbug.com/709069 -#if !defined(OS_MACOSX) -TEST(HeapDeathTest, MarkingSameThreadCheck) { - // This will crash during marking, at the DCHECK in Visitor::markHeader() or - // earlier. - EXPECT_DEATH(MarkingSameThreadCheckTester().Test(), ""); -} -#endif -#endif - static bool AllocateAndReturnBool() { ConservativelyCollectGarbage(); return true; } -static bool CheckGCForbidden() { - DCHECK(ThreadState::Current()->IsGCForbidden()); - return true; -} - -class MixinClass : public GarbageCollectedMixin { - public: - MixinClass() : dummy_(CheckGCForbidden()) {} - - private: - bool dummy_; -}; - -class ClassWithGarbageCollectingMixinConstructor - : public GarbageCollected<ClassWithGarbageCollectingMixinConstructor>, - public MixinClass { - USING_GARBAGE_COLLECTED_MIXIN(ClassWithGarbageCollectingMixinConstructor); - - public: - static int trace_called_; - - ClassWithGarbageCollectingMixinConstructor() - : trace_counter_(TraceCounter::Create()), - wrapper_(IntWrapper::Create(32)) {} - - void Trace(blink::Visitor* visitor) override { - trace_called_++; - visitor->Trace(trace_counter_); - visitor->Trace(wrapper_); - } - - void Verify() { - EXPECT_EQ(32, wrapper_->Value()); - EXPECT_EQ(0, trace_counter_->TraceCount()); - EXPECT_EQ(0, trace_called_); - } - - private: - Member<TraceCounter> trace_counter_; - Member<IntWrapper> wrapper_; -}; - -int ClassWithGarbageCollectingMixinConstructor::trace_called_ = 0; - -// Regression test for out of bounds call through vtable. -// Passes if it doesn't crash. -TEST(HeapTest, GarbageCollectionDuringMixinConstruction) { - ClassWithGarbageCollectingMixinConstructor::trace_called_ = 0; - ClassWithGarbageCollectingMixinConstructor* a = - new ClassWithGarbageCollectingMixinConstructor(); - a->Verify(); -} - -class DestructorLockingObject - : public GarbageCollectedFinalized<DestructorLockingObject> { - public: - static DestructorLockingObject* Create() { - return new DestructorLockingObject(); - } - - virtual ~DestructorLockingObject() { - ++destructor_calls_; - } - - static int destructor_calls_; - void Trace(blink::Visitor* visitor) {} - - private: - DestructorLockingObject() = default; -}; - -int DestructorLockingObject::destructor_calls_ = 0; - template <typename T> class TraceIfNeededTester : public GarbageCollectedFinalized<TraceIfNeededTester<T>> { @@ -5767,7 +5403,7 @@ class PartObject { class AllocatesOnAssignment { public: AllocatesOnAssignment(std::nullptr_t) : value_(nullptr) {} - AllocatesOnAssignment(int x) : value_(new IntWrapper(x)) {} + AllocatesOnAssignment(int x) : value_(MakeGarbageCollected<IntWrapper>(x)) {} AllocatesOnAssignment(IntWrapper* x) : value_(x) {} AllocatesOnAssignment& operator=(const AllocatesOnAssignment x) { @@ -5780,7 +5416,7 @@ class AllocatesOnAssignment { AllocatesOnAssignment(const AllocatesOnAssignment& other) { if (!ThreadState::Current()->IsGCForbidden()) ConservativelyCollectGarbage(); - value_ = new IntWrapper(other.value_->Value()); + value_ = MakeGarbageCollected<IntWrapper>(other.value_->Value()); } AllocatesOnAssignment(DeletedMarker) : value_(WTF::kHashTableDeletedValue) {} @@ -5852,7 +5488,7 @@ namespace blink { TEST(HeapTest, GCInHashMapOperations) { typedef HeapHashMap<AllocatesOnAssignment, AllocatesOnAssignment> Map; Map* map = MakeGarbageCollected<Map>(); - IntWrapper* key = new IntWrapper(42); + IntWrapper* key = MakeGarbageCollected<IntWrapper>(42); map->insert(key, AllocatesOnAssignment(103)); map->erase(key); for (int i = 0; i < 10; i++) @@ -5878,7 +5514,8 @@ class ObjectWithVirtualPartObject }; TEST(HeapTest, PartObjectWithVirtualMethod) { - ObjectWithVirtualPartObject* object = new ObjectWithVirtualPartObject(); + ObjectWithVirtualPartObject* object = + MakeGarbageCollected<ObjectWithVirtualPartObject>(); EXPECT_TRUE(object); } @@ -5905,7 +5542,7 @@ class AllocInSuperConstructorArgument // an object with an uninitialized vtable. TEST(HeapTest, AllocationInSuperConstructorArgument) { AllocInSuperConstructorArgument* object = - new AllocInSuperConstructorArgument(); + MakeGarbageCollected<AllocInSuperConstructorArgument>(); EXPECT_TRUE(object); ThreadState::Current()->CollectAllGarbage(); } @@ -5925,7 +5562,7 @@ class NonNodeAllocatingNodeInDestructor Persistent<IntNode>* NonNodeAllocatingNodeInDestructor::node_ = nullptr; TEST(HeapTest, NonNodeAllocatingNodeInDestructor) { - new NonNodeAllocatingNodeInDestructor(); + MakeGarbageCollected<NonNodeAllocatingNodeInDestructor>(); PreciselyCollectGarbage(); EXPECT_EQ(10, (*NonNodeAllocatingNodeInDestructor::node_)->Value()); delete NonNodeAllocatingNodeInDestructor::node_; @@ -5999,7 +5636,7 @@ TEST(HeapTest, TraceDeepEagerly) { #if !DCHECK_IS_ON() && !defined(OS_ANDROID) DeepEagerly* obj = nullptr; for (int i = 0; i < 10000000; i++) - obj = new DeepEagerly(obj); + obj = MakeGarbageCollected<DeepEagerly>(obj); Persistent<DeepEagerly> persistent(obj); PreciselyCollectGarbage(); @@ -6238,11 +5875,7 @@ class TestMixinAllocationA : public GarbageCollected<TestMixinAllocationA>, USING_GARBAGE_COLLECTED_MIXIN(TestMixinAllocationA); public: - TestMixinAllocationA() { - // Completely wrong in general, but test only - // runs this constructor while constructing another mixin. - DCHECK(ThreadState::Current()->IsGCForbidden()); - } + TestMixinAllocationA() = default; void Trace(blink::Visitor* visitor) override {} }; @@ -6252,13 +5885,8 @@ class TestMixinAllocationB : public TestMixinAllocationA { public: TestMixinAllocationB() - : a_(new TestMixinAllocationA()) // Construct object during a mixin - // construction. - { - // Completely wrong in general, but test only - // runs this constructor while constructing another mixin. - DCHECK(ThreadState::Current()->IsGCForbidden()); - } + // Construct object during a mixin construction. + : a_(MakeGarbageCollected<TestMixinAllocationA>()) {} void Trace(blink::Visitor* visitor) override { visitor->Trace(a_); @@ -6281,7 +5909,7 @@ class TestMixinAllocationC final : public TestMixinAllocationB { }; TEST(HeapTest, NestedMixinConstruction) { - TestMixinAllocationC* object = new TestMixinAllocationC(); + TestMixinAllocationC* object = MakeGarbageCollected<TestMixinAllocationC>(); EXPECT_TRUE(object); } @@ -6303,47 +5931,6 @@ class ObjectWithLargeAmountsOfAllocationInConstructor { } }; -class TestMixinAllocatingObject final - : public TestMixinAllocationB, - public ObjectWithLargeAmountsOfAllocationInConstructor { - USING_GARBAGE_COLLECTED_MIXIN(TestMixinAllocatingObject); - - public: - static TestMixinAllocatingObject* Create(ClassWithMember* member) { - return new TestMixinAllocatingObject(member); - } - - void Trace(blink::Visitor* visitor) override { - visitor->Trace(trace_counter_); - TestMixinAllocationB::Trace(visitor); - } - - int TraceCount() const { return trace_counter_->TraceCount(); } - - private: - TestMixinAllocatingObject(ClassWithMember* member) - : ObjectWithLargeAmountsOfAllocationInConstructor(600, member), - trace_counter_(TraceCounter::Create()) { - DCHECK(!ThreadState::Current()->IsGCForbidden()); - ConservativelyCollectGarbage(); - EXPECT_GT(member->TraceCount(), 0); - EXPECT_GT(TraceCount(), 0); - } - - Member<TraceCounter> trace_counter_; -}; - -TEST(HeapTest, MixinConstructionNoGC) { - ClearOutOldGarbage(); - Persistent<ClassWithMember> object = ClassWithMember::Create(); - EXPECT_EQ(0, object->TraceCount()); - TestMixinAllocatingObject* mixin = - TestMixinAllocatingObject::Create(object.Get()); - EXPECT_TRUE(mixin); - EXPECT_GT(object->TraceCount(), 0); - EXPECT_GT(mixin->TraceCount(), 0); -} - class WeakPersistentHolder final { public: explicit WeakPersistentHolder(IntWrapper* object) : object_(object) {} @@ -6354,7 +5941,7 @@ class WeakPersistentHolder final { }; TEST(HeapTest, WeakPersistent) { - Persistent<IntWrapper> object = new IntWrapper(20); + Persistent<IntWrapper> object = MakeGarbageCollected<IntWrapper>(20); std::unique_ptr<WeakPersistentHolder> holder = std::make_unique<WeakPersistentHolder>(object); PreciselyCollectGarbage(); @@ -6366,71 +5953,6 @@ TEST(HeapTest, WeakPersistent) { namespace { -void WorkerThreadMainForCrossThreadWeakPersistentTest( - DestructorLockingObject** object) { - // Step 2: Create an object and store the pointer. - MutexLocker locker(WorkerThreadMutex()); - ThreadState::AttachCurrentThread(); - *object = DestructorLockingObject::Create(); - WakeMainThread(); - ParkWorkerThread(); - - // Step 4: Run a GC. - ThreadState::Current()->CollectGarbage( - BlinkGC::kNoHeapPointersOnStack, BlinkGC::kAtomicMarking, - BlinkGC::kEagerSweeping, BlinkGC::GCReason::kForcedGC); - WakeMainThread(); - ParkWorkerThread(); - - // Step 6: Finish. - ThreadState::DetachCurrentThread(); - WakeMainThread(); -} - -} // anonymous namespace - -TEST(HeapTest, CrossThreadWeakPersistent) { - // Create an object in the worker thread, have a CrossThreadWeakPersistent - // pointing to it on the main thread, clear the reference in the worker - // thread, run a GC in the worker thread, and see if the - // CrossThreadWeakPersistent is cleared. - - DestructorLockingObject::destructor_calls_ = 0; - - // Step 1: Initiate a worker thread, and wait for |object| to get allocated on - // the worker thread. - MutexLocker main_thread_mutex_locker(MainThreadMutex()); - std::unique_ptr<Thread> worker_thread = Platform::Current()->CreateThread( - ThreadCreationParams(WebThreadType::kTestThread) - .SetThreadNameForTest("Test Worker Thread")); - DestructorLockingObject* object = nullptr; - PostCrossThreadTask( - *worker_thread->GetTaskRunner(), FROM_HERE, - CrossThreadBind(WorkerThreadMainForCrossThreadWeakPersistentTest, - CrossThreadUnretained(&object))); - ParkMainThread(); - - // Step 3: Set up a CrossThreadWeakPersistent. - ASSERT_TRUE(object); - CrossThreadWeakPersistent<DestructorLockingObject> - cross_thread_weak_persistent(object); - object = nullptr; - EXPECT_EQ(0, DestructorLockingObject::destructor_calls_); - - // Pretend we have no pointers on stack during the step 4. - WakeWorkerThread(); - ParkMainThread(); - - // Step 5: Make sure the weak persistent is cleared. - EXPECT_FALSE(cross_thread_weak_persistent.Get()); - EXPECT_EQ(1, DestructorLockingObject::destructor_calls_); - - WakeWorkerThread(); - ParkMainThread(); -} - -namespace { - class ThreadedClearOnShutdownTester : public ThreadedTesterBase { public: static void Test() { @@ -6443,11 +5965,8 @@ class ThreadedClearOnShutdownTester : public ThreadedTesterBase { void RunWhileAttached(); void RunThread() override { - ThreadState::AttachCurrentThread(); EXPECT_EQ(42, ThreadSpecificIntWrapper().Value()); RunWhileAttached(); - ThreadState::DetachCurrentThread(); - AtomicDecrement(&threads_to_finish_); } class HeapObject; @@ -6465,7 +5984,7 @@ class ThreadedClearOnShutdownTester : public ThreadedTesterBase { int_wrapper, ()); Persistent<IntWrapper>& handle = *int_wrapper; if (!handle) { - handle = new IntWrapper(42); + handle = MakeGarbageCollected<IntWrapper>(42); handle.RegisterAsStaticReference(); } return *handle; @@ -6477,9 +5996,11 @@ class ThreadedClearOnShutdownTester::HeapObject final ThreadedClearOnShutdownTester::HeapObject> { public: static HeapObject* Create(bool test_destructor) { - return new HeapObject(test_destructor); + return MakeGarbageCollected<HeapObject>(test_destructor); } + explicit HeapObject(bool test_destructor) + : test_destructor_(test_destructor) {} ~HeapObject() { if (!test_destructor_) return; @@ -6498,9 +6019,6 @@ class ThreadedClearOnShutdownTester::HeapObject final void Trace(blink::Visitor* visitor) {} private: - explicit HeapObject(bool test_destructor) - : test_destructor_(test_destructor) {} - bool test_destructor_; }; @@ -6544,16 +6062,16 @@ TEST(HeapTest, TestClearOnShutdown) { class WithWeakConstObject final : public GarbageCollected<WithWeakConstObject> { public: static WithWeakConstObject* Create(const IntWrapper* int_wrapper) { - return new WithWeakConstObject(int_wrapper); + return MakeGarbageCollected<WithWeakConstObject>(int_wrapper); } + WithWeakConstObject(const IntWrapper* int_wrapper) : wrapper_(int_wrapper) {} + void Trace(blink::Visitor* visitor) { visitor->Trace(wrapper_); } const IntWrapper* Value() const { return wrapper_; } private: - WithWeakConstObject(const IntWrapper* int_wrapper) : wrapper_(int_wrapper) {} - WeakMember<const IntWrapper> wrapper_; }; @@ -6649,7 +6167,7 @@ class DoublyLinkedListNodeImpl public: DoublyLinkedListNodeImpl() = default; static DoublyLinkedListNodeImpl* Create() { - return new DoublyLinkedListNodeImpl(); + return MakeGarbageCollected<DoublyLinkedListNodeImpl>(); } static int destructor_calls_; @@ -6673,7 +6191,7 @@ class HeapDoublyLinkedListContainer : public GarbageCollected<HeapDoublyLinkedListContainer<T>> { public: static HeapDoublyLinkedListContainer<T>* Create() { - return new HeapDoublyLinkedListContainer<T>(); + return MakeGarbageCollected<HeapDoublyLinkedListContainer<T>>(); } HeapDoublyLinkedListContainer<T>() = default; HeapDoublyLinkedList<T> list_; @@ -6706,7 +6224,7 @@ TEST(HeapTest, PromptlyFreeStackAllocatedHeapVector) { Address before; { HeapVector<Member<IntWrapper>> vector; - vector.push_back(new IntWrapper(0)); + vector.push_back(MakeGarbageCollected<IntWrapper>(0)); NormalPage* normal_page = static_cast<NormalPage*>(PageFromObject(vector.data())); normal_arena = normal_page->ArenaForNormalPage(); @@ -6723,7 +6241,7 @@ TEST(HeapTest, PromptlyFreeStackAllocatedHeapDeque) { Address before; { HeapDeque<Member<IntWrapper>> deque; - deque.push_back(new IntWrapper(0)); + deque.push_back(MakeGarbageCollected<IntWrapper>(0)); NormalPage* normal_page = static_cast<NormalPage*>(PageFromObject(&deque.front())); normal_arena = normal_page->ArenaForNormalPage(); @@ -6742,7 +6260,7 @@ TEST(HeapTest, PromptlyFreeStackAllocatedHeapHashSet) { Address before; { HeapHashSet<Member<IntWrapper>> hash_set; - hash_set.insert(new IntWrapper(0)); + hash_set.insert(MakeGarbageCollected<IntWrapper>(0)); before = normal_arena->CurrentAllocationPoint(); } Address after = normal_arena->CurrentAllocationPoint(); @@ -6758,7 +6276,7 @@ TEST(HeapTest, PromptlyFreeStackAllocatedHeapListHashSet) { Address before; { HeapListHashSet<Member<IntWrapper>> list_hash_set; - list_hash_set.insert(new IntWrapper(0)); + list_hash_set.insert(MakeGarbageCollected<IntWrapper>(0)); before = normal_arena->CurrentAllocationPoint(); } Address after = normal_arena->CurrentAllocationPoint(); @@ -6773,7 +6291,7 @@ TEST(HeapTest, PromptlyFreeStackAllocatedHeapLinkedHashSet) { Address before; { HeapLinkedHashSet<Member<IntWrapper>> linked_hash_set; - linked_hash_set.insert(new IntWrapper(0)); + linked_hash_set.insert(MakeGarbageCollected<IntWrapper>(0)); before = normal_arena->CurrentAllocationPoint(); } Address after = normal_arena->CurrentAllocationPoint(); @@ -6787,7 +6305,7 @@ TEST(HeapTest, ShrinkVector) { HeapVector<Member<IntWrapper>> vector; vector.ReserveCapacity(32); for (int i = 0; i < 4; i++) { - vector.push_back(new IntWrapper(i)); + vector.push_back(MakeGarbageCollected<IntWrapper>(i)); } ConservativelyCollectGarbage(BlinkGC::kLazySweeping); @@ -6798,85 +6316,30 @@ TEST(HeapTest, ShrinkVector) { vector.ShrinkToFit(); } -namespace { - -class MixinCheckingConstructionScope : public GarbageCollectedMixin { - public: - MixinCheckingConstructionScope() { - // Oilpan treats mixin construction as forbidden scopes for garbage - // collection. - CHECK(ThreadState::Current()->IsMixinInConstruction()); - } -}; - -class UsingMixinCheckingConstructionScope - : public GarbageCollected<UsingMixinCheckingConstructionScope>, - public MixinCheckingConstructionScope { - USING_GARBAGE_COLLECTED_MIXIN(UsingMixinCheckingConstructionScope); -}; - -} // namespace - -TEST(HeapTest, NoConservativeGCDuringMixinConstruction) { - // Regression test: https://crbug.com/904546 - MakeGarbageCollected<UsingMixinCheckingConstructionScope>(); +TEST(HeapTest, GarbageCollectedInConstruction) { + using O = ObjectWithCallbackBeforeInitializer<IntWrapper>; + MakeGarbageCollected<O>(base::BindOnce([](O* thiz) { + CHECK(HeapObjectHeader::FromPayload(thiz)->IsInConstruction()); + })); } -namespace { - -class ObjectCheckingForInConstruction - : public GarbageCollected<ObjectCheckingForInConstruction> { - public: - ObjectCheckingForInConstruction() { - CHECK(HeapObjectHeader::FromPayload(this)->IsInConstruction()); - } - - virtual void Trace(Visitor* v) { v->Trace(foo_); } - - private: - Member<IntWrapper> foo_; -}; - -class MixinCheckingInConstruction : public GarbageCollectedMixin { - public: - MixinCheckingInConstruction() { - BasePage* const page = PageFromObject(reinterpret_cast<Address>(this)); +TEST(HeapTest, GarbageCollectedMixinInConstruction) { + using O = ObjectWithMixinWithCallbackBeforeInitializer<IntWrapper>; + MakeGarbageCollected<O>(base::BindOnce([](O::Mixin* thiz) { + BasePage* const page = PageFromObject(thiz); HeapObjectHeader* const header = - static_cast<NormalPage*>(page)->FindHeaderFromAddress( - reinterpret_cast<Address>( - const_cast<MixinCheckingInConstruction*>(this))); + page->IsLargeObjectPage() + ? static_cast<LargeObjectPage*>(page)->ObjectHeader() + : static_cast<NormalPage*>(page)->FindHeaderFromAddress( + reinterpret_cast<Address>(thiz)); CHECK(header->IsInConstruction()); - } - - void Trace(Visitor* v) override { v->Trace(bar_); } - - private: - Member<IntWrapper> bar_; -}; - -class MixinAppCheckingInConstruction - : public GarbageCollected<MixinAppCheckingInConstruction>, - public MixinCheckingInConstruction { - USING_GARBAGE_COLLECTED_MIXIN(MixinAppCheckingInConstruction) - public: - MixinAppCheckingInConstruction() { - CHECK(HeapObjectHeader::FromPayload(this)->IsInConstruction()); - } - - void Trace(Visitor* v) override { v->Trace(foo_); } - - private: - Member<IntWrapper> foo_; -}; - -} // namespace - -TEST(HeapTest, GarbageCollectedInConstruction) { - MakeGarbageCollected<ObjectCheckingForInConstruction>(); + })); } -TEST(HeapTest, GarbageCollectedMixinInConstruction) { - MakeGarbageCollected<MixinAppCheckingInConstruction>(); +TEST(HeapTest, GarbageCollectedMixinIsAliveDuringConstruction) { + using O = ObjectWithMixinWithCallbackBeforeInitializer<IntWrapper>; + MakeGarbageCollected<O>(base::BindOnce( + [](O::Mixin* thiz) { CHECK(ThreadHeap::IsHeapObjectAlive(thiz)); })); } } // namespace blink diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_test_utilities.h b/chromium/third_party/blink/renderer/platform/heap/heap_test_utilities.h index 97778731db1..31af7270aca 100644 --- a/chromium/third_party/blink/renderer/platform/heap/heap_test_utilities.h +++ b/chromium/third_party/blink/renderer/platform/heap/heap_test_utilities.h @@ -7,7 +7,12 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_HEAP_TEST_UTILITIES_H_ #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_HEAP_TEST_UTILITIES_H_ +#include "base/callback.h" #include "third_party/blink/renderer/platform/heap/blink_gc.h" +#include "third_party/blink/renderer/platform/heap/garbage_collected.h" +#include "third_party/blink/renderer/platform/heap/heap.h" +#include "third_party/blink/renderer/platform/heap/trace_traits.h" +#include "third_party/blink/renderer/platform/heap/visitor.h" namespace blink { @@ -16,6 +21,90 @@ void ConservativelyCollectGarbage( BlinkGC::SweepingType sweeping_type = BlinkGC::kEagerSweeping); void ClearOutOldGarbage(); +template <typename T> +class ObjectWithCallbackBeforeInitializer + : public GarbageCollected<ObjectWithCallbackBeforeInitializer<T>> { + public: + ObjectWithCallbackBeforeInitializer( + base::OnceCallback<void(ObjectWithCallbackBeforeInitializer<T>*)>&& cb, + T* value) + : bool_(ExecuteCallbackReturnTrue(this, std::move(cb))), value_(value) {} + + ObjectWithCallbackBeforeInitializer( + base::OnceCallback<void(ObjectWithCallbackBeforeInitializer<T>*)>&& cb) + : bool_(ExecuteCallbackReturnTrue(this, std::move(cb))) {} + + virtual void Trace(Visitor* visitor) { visitor->Trace(value_); } + + T* value() const { return value_.Get(); } + + private: + static bool ExecuteCallbackReturnTrue( + ObjectWithCallbackBeforeInitializer* thiz, + base::OnceCallback<void(ObjectWithCallbackBeforeInitializer<T>*)>&& cb) { + std::move(cb).Run(thiz); + return true; + } + + bool bool_; + Member<T> value_; +}; + +template <typename T> +class MixinWithCallbackBeforeInitializer : public GarbageCollectedMixin { + public: + MixinWithCallbackBeforeInitializer( + base::OnceCallback<void(MixinWithCallbackBeforeInitializer<T>*)>&& cb, + T* value) + : bool_(ExecuteCallbackReturnTrue(this, std::move(cb))), value_(value) {} + + MixinWithCallbackBeforeInitializer( + base::OnceCallback<void(MixinWithCallbackBeforeInitializer<T>*)>&& cb) + : bool_(ExecuteCallbackReturnTrue(this, std::move(cb))) {} + + void Trace(Visitor* visitor) override { visitor->Trace(value_); } + + T* value() const { return value_.Get(); } + + private: + static bool ExecuteCallbackReturnTrue( + MixinWithCallbackBeforeInitializer* thiz, + base::OnceCallback<void(MixinWithCallbackBeforeInitializer<T>*)>&& cb) { + std::move(cb).Run(thiz); + return true; + } + + bool bool_; + Member<T> value_; +}; + +class BoolMixin { + protected: + bool bool_ = false; +}; + +template <typename T> +class ObjectWithMixinWithCallbackBeforeInitializer + : public GarbageCollected<ObjectWithMixinWithCallbackBeforeInitializer<T>>, + public BoolMixin, + public MixinWithCallbackBeforeInitializer<T> { + USING_GARBAGE_COLLECTED_MIXIN(ObjectWithMixinWithCallbackBeforeInitializer); + + public: + using Mixin = MixinWithCallbackBeforeInitializer<T>; + + ObjectWithMixinWithCallbackBeforeInitializer( + base::OnceCallback<void(Mixin*)>&& cb, + T* value) + : Mixin(std::move(cb), value) {} + + ObjectWithMixinWithCallbackBeforeInitializer( + base::OnceCallback<void(Mixin*)>&& cb) + : Mixin(std::move(cb)) {} + + void Trace(Visitor* visitor) override { Mixin::Trace(visitor); } +}; + } // namespace blink #endif // THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_HEAP_TEST_UTILITIES_H_ diff --git a/chromium/third_party/blink/renderer/platform/heap/heap_thread_test.cc b/chromium/third_party/blink/renderer/platform/heap/heap_thread_test.cc new file mode 100644 index 00000000000..b1f86ce0cee --- /dev/null +++ b/chromium/third_party/blink/renderer/platform/heap/heap_thread_test.cc @@ -0,0 +1,269 @@ +// Copyright 2018 The Chromium 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 "build/build_config.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/blink/public/platform/platform.h" +#include "third_party/blink/renderer/platform/cross_thread_functional.h" +#include "third_party/blink/renderer/platform/heap/handle.h" +#include "third_party/blink/renderer/platform/heap/heap_test_utilities.h" +#include "third_party/blink/renderer/platform/heap/thread_state.h" +#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h" +#include "third_party/blink/renderer/platform/scheduler/public/thread.h" + +namespace blink { +namespace heap_thread_test { + +static Mutex& ActiveThreadMutex() { + DEFINE_THREAD_SAFE_STATIC_LOCAL(Mutex, active_thread_mutex, ()); + return active_thread_mutex; +} + +static ThreadCondition& ActiveThreadCondition() { + DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadCondition, active_thread_condition, + (ActiveThreadMutex())); + return active_thread_condition; +} + +enum ActiveThreadState { + kNoThreadActive, + kMainThreadActive, + kWorkerThreadActive, +}; + +static ActiveThreadState& ActiveThread() { + DEFINE_THREAD_SAFE_STATIC_LOCAL(ActiveThreadState, active_thread, + (kNoThreadActive)); + return active_thread; +} + +static void WakeMainThread() { + ActiveThread() = kMainThreadActive; + ActiveThreadCondition().Signal(); +} + +static void WakeWorkerThread() { + ActiveThread() = kWorkerThreadActive; + ActiveThreadCondition().Signal(); +} + +static void ParkMainThread() { + while (ActiveThread() != kMainThreadActive) { + ActiveThreadCondition().Wait(); + } +} + +static void ParkWorkerThread() { + while (ActiveThread() != kWorkerThreadActive) { + ActiveThreadCondition().Wait(); + } +} + +class Object : public GarbageCollected<Object> { + public: + Object() {} + ~Object() {} + void Trace(blink::Visitor* visitor) {} +}; + +class AlternatingThreadTester { + public: + void Test() { + MutexLocker locker(ActiveThreadMutex()); + ActiveThread() = kMainThreadActive; + + std::unique_ptr<Thread> worker_thread = Platform::Current()->CreateThread( + ThreadCreationParams(WebThreadType::kTestThread) + .SetThreadNameForTest("Test Worker Thread")); + PostCrossThreadTask( + *worker_thread->GetTaskRunner(), FROM_HERE, + CrossThreadBind(&AlternatingThreadTester::StartWorkerThread, + CrossThreadUnretained(this))); + + MainThreadMain(); + } + + void SwitchToWorkerThread() { + WakeWorkerThread(); + ParkMainThread(); + } + + void SwitchToMainThread() { + WakeMainThread(); + ParkWorkerThread(); + } + + protected: + // Override with code you want to execute on the main thread. + virtual void MainThreadMain() = 0; + // Override with code you want to execute on the worker thread. At the end, + // the ThreadState is detached and we switch back to the main thread + // automatically. + virtual void WorkerThreadMain() = 0; + + private: + void StartWorkerThread() { + ThreadState::AttachCurrentThread(); + + MutexLocker locker(ActiveThreadMutex()); + + WorkerThreadMain(); + + ThreadState::DetachCurrentThread(); + WakeMainThread(); + } +}; + +class MemberSameThreadCheckTester : public AlternatingThreadTester { + private: + void MainThreadMain() override { SwitchToWorkerThread(); } + + void WorkerThreadMain() override { + // Setting an object created on the worker thread to a Member allocated on + // the main thread is not allowed. + object_ = MakeGarbageCollected<Object>(); + } + + Member<Object> object_; +}; + +#if DCHECK_IS_ON() +// TODO(keishi) This test is flaky on mac_chromium_rel_ng bot. +// crbug.com/709069 +#if !defined(OS_MACOSX) +TEST(HeapDeathTest, MemberSameThreadCheck) { + EXPECT_DEATH(MemberSameThreadCheckTester().Test(), ""); +} +#endif +#endif + +class PersistentSameThreadCheckTester : public AlternatingThreadTester { + private: + void MainThreadMain() override { SwitchToWorkerThread(); } + + void WorkerThreadMain() override { + // Setting an object created on the worker thread to a Persistent allocated + // on the main thread is not allowed. + object_ = MakeGarbageCollected<Object>(); + } + + Persistent<Object> object_; +}; + +#if DCHECK_IS_ON() +// TODO(keishi) This test is flaky on mac_chromium_rel_ng bot. +// crbug.com/709069 +#if !defined(OS_MACOSX) +TEST(HeapDeathTest, PersistentSameThreadCheck) { + EXPECT_DEATH(PersistentSameThreadCheckTester().Test(), ""); +} +#endif +#endif + +class MarkingSameThreadCheckTester : public AlternatingThreadTester { + private: + class MainThreadObject : public GarbageCollectedFinalized<MainThreadObject> { + public: + void Trace(blink::Visitor* visitor) { visitor->Trace(set_); } + void AddToSet(Object* object) { set_.insert(42, object); } + + private: + HeapHashMap<int, Member<Object>> set_; + }; + + void MainThreadMain() override { + main_thread_object_ = MakeGarbageCollected<MainThreadObject>(); + + SwitchToWorkerThread(); + + // This will try to mark MainThreadObject when it tries to mark Object + // it should crash. + PreciselyCollectGarbage(); + } + + void WorkerThreadMain() override { + // Adding a reference to an object created on the worker thread to a + // HeapHashMap created on the main thread is not allowed. + main_thread_object_->AddToSet(MakeGarbageCollected<Object>()); + } + + CrossThreadPersistent<MainThreadObject> main_thread_object_; +}; + +#if DCHECK_IS_ON() +// TODO(keishi) This test is flaky on mac_chromium_rel_ng bot. +// crbug.com/709069 +#if !defined(OS_MACOSX) +TEST(HeapDeathTest, MarkingSameThreadCheck) { + // This will crash during marking, at the DCHECK in Visitor::markHeader() or + // earlier. + EXPECT_DEATH(MarkingSameThreadCheckTester().Test(), ""); +} +#endif +#endif + +class DestructorLockingObject + : public GarbageCollectedFinalized<DestructorLockingObject> { + public: + static DestructorLockingObject* Create() { + return MakeGarbageCollected<DestructorLockingObject>(); + } + + DestructorLockingObject() = default; + virtual ~DestructorLockingObject() { ++destructor_calls_; } + + static int destructor_calls_; + void Trace(blink::Visitor* visitor) {} +}; + +int DestructorLockingObject::destructor_calls_ = 0; + +class CrossThreadWeakPersistentTester : public AlternatingThreadTester { + private: + void MainThreadMain() override { + // Create an object in the worker thread, have a CrossThreadWeakPersistent + // pointing to it on the main thread, run a GC in the worker thread, and see + // if the CrossThreadWeakPersistent is cleared. + + DestructorLockingObject::destructor_calls_ = 0; + + // Step 1: Initiate a worker thread, and wait for |Object| to get allocated + // on the worker thread. + SwitchToWorkerThread(); + + // Step 3: Set up a CrossThreadWeakPersistent. + ASSERT_TRUE(object_); + EXPECT_EQ(0, DestructorLockingObject::destructor_calls_); + + // Pretend we have no pointers on stack during the step 4. + SwitchToWorkerThread(); + + // Step 5: Make sure the weak persistent is cleared. + EXPECT_FALSE(object_.Get()); + EXPECT_EQ(1, DestructorLockingObject::destructor_calls_); + + SwitchToWorkerThread(); + } + + void WorkerThreadMain() override { + // Step 2: Create an object and store the pointer. + object_ = DestructorLockingObject::Create(); + SwitchToMainThread(); + + // Step 4: Run a GC. + ThreadState::Current()->CollectGarbage( + BlinkGC::kNoHeapPointersOnStack, BlinkGC::kAtomicMarking, + BlinkGC::kEagerSweeping, BlinkGC::GCReason::kForcedGC); + SwitchToMainThread(); + } + + CrossThreadWeakPersistent<DestructorLockingObject> object_; +}; + +TEST(HeapThreadTest, CrossThreadWeakPersistent) { + CrossThreadWeakPersistentTester().Test(); +} + +} // namespace heap_thread_test +} // namespace blink diff --git a/chromium/third_party/blink/renderer/platform/heap/incremental_marking_test.cc b/chromium/third_party/blink/renderer/platform/heap/incremental_marking_test.cc index f82eaf2df9c..b359297ba28 100644 --- a/chromium/third_party/blink/renderer/platform/heap/incremental_marking_test.cc +++ b/chromium/third_party/blink/renderer/platform/heap/incremental_marking_test.cc @@ -223,8 +223,13 @@ class ExpectNoWriteBarrierFires : public IncrementalMarkingScope { class Object : public GarbageCollected<Object> { public: - static Object* Create() { return new Object(); } - static Object* Create(Object* next) { return new Object(next); } + static Object* Create() { return MakeGarbageCollected<Object>(); } + static Object* Create(Object* next) { + return MakeGarbageCollected<Object>(next); + } + + Object() : next_(nullptr) {} + explicit Object(Object* next) : next_(next) {} void set_next(Object* next) { next_ = next; } @@ -237,9 +242,6 @@ class Object : public GarbageCollected<Object> { Member<Object>& next_ref() { return next_; } private: - Object() : next_(nullptr) {} - explicit Object(Object* next) : next_(next) {} - Member<Object> next_; }; @@ -389,31 +391,30 @@ class Child : public GarbageCollected<Child>, USING_GARBAGE_COLLECTED_MIXIN(Child); public: - static Child* Create() { return new Child(); } + static Child* Create() { return MakeGarbageCollected<Child>(); } + + Child() : ClassWithVirtual(), Mixin() {} ~Child() override {} void Trace(blink::Visitor* visitor) override { Mixin::Trace(visitor); } void Foo() override {} void Bar() override {} - - protected: - Child() : ClassWithVirtual(), Mixin() {} }; class ParentWithMixinPointer : public GarbageCollected<ParentWithMixinPointer> { public: static ParentWithMixinPointer* Create() { - return new ParentWithMixinPointer(); + return MakeGarbageCollected<ParentWithMixinPointer>(); } + ParentWithMixinPointer() : mixin_(nullptr) {} + void set_mixin(Mixin* mixin) { mixin_ = mixin; } virtual void Trace(blink::Visitor* visitor) { visitor->Trace(mixin_); } protected: - ParentWithMixinPointer() : mixin_(nullptr) {} - Member<Mixin> mixin_; }; @@ -700,7 +701,7 @@ class ObjectNode : public GarbageCollected<ObjectNode>, TEST(IncrementalMarkingTest, HeapDoublyLinkedListPush) { Object* obj = Object::Create(); - ObjectNode* obj_node = new ObjectNode(obj); + ObjectNode* obj_node = MakeGarbageCollected<ObjectNode>(obj); HeapDoublyLinkedList<ObjectNode> list; { ExpectWriteBarrierFires scope(ThreadState::Current(), {obj_node}); @@ -712,7 +713,7 @@ TEST(IncrementalMarkingTest, HeapDoublyLinkedListPush) { TEST(IncrementalMarkingTest, HeapDoublyLinkedListAppend) { Object* obj = Object::Create(); - ObjectNode* obj_node = new ObjectNode(obj); + ObjectNode* obj_node = MakeGarbageCollected<ObjectNode>(obj); HeapDoublyLinkedList<ObjectNode> list; { ExpectWriteBarrierFires scope(ThreadState::Current(), {obj_node}); @@ -1502,7 +1503,8 @@ class RegisteringObject : public GarbageCollected<RegisteringObject>, TEST(IncrementalMarkingTest, WriteBarrierDuringMixinConstruction) { IncrementalMarkingScope scope(ThreadState::Current()); ObjectRegistry registry; - RegisteringObject* object = new RegisteringObject(®istry); + RegisteringObject* object = + MakeGarbageCollected<RegisteringObject>(®istry); // Clear any objects that have been added to the regular marking worklist in // the process of calling the constructor. @@ -1536,7 +1538,7 @@ TEST(IncrementalMarkingTest, WriteBarrierDuringMixinConstruction) { TEST(IncrementalMarkingTest, OverrideAfterMixinConstruction) { ObjectRegistry registry; - RegisteringMixin* mixin = new RegisteringObject(®istry); + RegisteringMixin* mixin = MakeGarbageCollected<RegisteringObject>(®istry); HeapObjectHeader* header = mixin->GetHeapObjectHeader(); const void* uninitialized_value = BlinkGC::kNotFullyConstructedObject; EXPECT_NE(uninitialized_value, header); @@ -1561,28 +1563,30 @@ class IncrementalMarkingTestDriver { thread_state_->IncrementalMarkingStart(BlinkGC::GCReason::kTesting); } - bool SingleStep() { + bool SingleStep(BlinkGC::StackState stack_state = + BlinkGC::StackState::kNoHeapPointersOnStack) { CHECK(thread_state_->IsIncrementalMarking()); if (thread_state_->GetGCState() == ThreadState::kIncrementalMarkingStepScheduled) { - thread_state_->RunScheduledGC(BlinkGC::kNoHeapPointersOnStack); + thread_state_->IncrementalMarkingStep(stack_state); return true; } return false; } - void FinishSteps() { + void FinishSteps(BlinkGC::StackState stack_state = + BlinkGC::StackState::kNoHeapPointersOnStack) { CHECK(thread_state_->IsIncrementalMarking()); - while (SingleStep()) { + while (SingleStep(stack_state)) { } } void FinishGC() { CHECK(thread_state_->IsIncrementalMarking()); - FinishSteps(); + FinishSteps(BlinkGC::StackState::kNoHeapPointersOnStack); CHECK_EQ(ThreadState::kIncrementalMarkingFinalizeScheduled, thread_state_->GetGCState()); - thread_state_->RunScheduledGC(BlinkGC::kNoHeapPointersOnStack); + thread_state_->RunScheduledGC(BlinkGC::StackState::kNoHeapPointersOnStack); CHECK(!thread_state_->IsIncrementalMarking()); thread_state_->CompleteSweep(); } @@ -1728,10 +1732,28 @@ TEST(IncrementalMarkingTest, WeakHashMapHeapCompaction) { persistent->insert(Object::Create()); driver.FinishGC(); - // Weak caallback should register the slot. + // Weak callback should register the slot. EXPECT_EQ(driver.GetHeapCompactLastFixupCount(), 1u); } +TEST(IncrementalMarkingTest, ConservativeGCWhileCompactionScheduled) { + using Store = HeapVector<Member<Object>>; + Persistent<Store> persistent(MakeGarbageCollected<Store>()); + persistent->push_back(Object::Create()); + + IncrementalMarkingTestDriver driver(ThreadState::Current()); + HeapCompact::ScheduleCompactionGCForTesting(true); + driver.Start(); + driver.FinishSteps(); + ThreadState::Current()->CollectGarbage( + BlinkGC::kHeapPointersOnStack, BlinkGC::kAtomicMarking, + BlinkGC::kLazySweeping, BlinkGC::GCReason::kConservativeGC); + + // Heap compaction should be canceled if incremental marking finishes with a + // conservative GC. + EXPECT_EQ(driver.GetHeapCompactLastFixupCount(), 0u); +} + namespace { class ObjectWithWeakMember : public GarbageCollected<ObjectWithWeakMember> { @@ -1781,6 +1803,80 @@ TEST(IncrementalMarkingTest, MemberSwap) { ConservativelyCollectGarbage(); } +namespace { + +template <typename T> +class ObjectHolder : public GarbageCollected<ObjectHolder<T>> { + public: + ObjectHolder() = default; + + virtual void Trace(Visitor* visitor) { visitor->Trace(holder_); } + + void set_value(T* value) { holder_ = value; } + T* value() const { return holder_.Get(); } + + private: + Member<T> holder_; +}; + +} // namespace + +TEST(IncrementalMarkingTest, StepDuringObjectConstruction) { + // Test ensures that objects in construction are delayed for processing to + // allow omitting write barriers on initializing stores. + + using O = ObjectWithCallbackBeforeInitializer<Object>; + using Holder = ObjectHolder<O>; + Persistent<Holder> holder(MakeGarbageCollected<Holder>()); + IncrementalMarkingTestDriver driver(ThreadState::Current()); + driver.Start(); + MakeGarbageCollected<O>( + base::BindOnce( + [](IncrementalMarkingTestDriver* driver, Holder* holder, O* thiz) { + // Publish not-fully-constructed object |thiz| by triggering write + // barrier for the object. + holder->set_value(thiz); + CHECK(HeapObjectHeader::FromPayload(holder->value())->IsValid()); + // Finish call incremental steps. + driver->FinishSteps(BlinkGC::StackState::kHeapPointersOnStack); + }, + &driver, holder.Get()), + MakeGarbageCollected<Object>()); + driver.FinishGC(); + CHECK(HeapObjectHeader::FromPayload(holder->value())->IsValid()); + CHECK(HeapObjectHeader::FromPayload(holder->value()->value())->IsValid()); + PreciselyCollectGarbage(); +} + +TEST(IncrementalMarkingTest, StepDuringMixinObjectConstruction) { + // Test ensures that mixin objects in construction are delayed for processing + // to allow omitting write barriers on initializing stores. + + using Parent = ObjectWithMixinWithCallbackBeforeInitializer<Object>; + using Mixin = MixinWithCallbackBeforeInitializer<Object>; + using Holder = ObjectHolder<Mixin>; + Persistent<Holder> holder(MakeGarbageCollected<Holder>()); + IncrementalMarkingTestDriver driver(ThreadState::Current()); + driver.Start(); + MakeGarbageCollected<Parent>( + base::BindOnce( + [](IncrementalMarkingTestDriver* driver, Holder* holder, + Mixin* thiz) { + // Publish not-fully-constructed object + // |thiz| by triggering write barrier for + // the object. + holder->set_value(thiz); + // Finish call incremental steps. + driver->FinishSteps(BlinkGC::StackState::kHeapPointersOnStack); + }, + &driver, holder.Get()), + MakeGarbageCollected<Object>()); + driver.FinishGC(); + CHECK(holder->value()->GetHeapObjectHeader()->IsValid()); + CHECK(HeapObjectHeader::FromPayload(holder->value()->value())->IsValid()); + PreciselyCollectGarbage(); +} + } // namespace incremental_marking_test } // namespace blink diff --git a/chromium/third_party/blink/renderer/platform/heap/marking_visitor.cc b/chromium/third_party/blink/renderer/platform/heap/marking_visitor.cc index d803593d36d..b95eb732fcc 100644 --- a/chromium/third_party/blink/renderer/platform/heap/marking_visitor.cc +++ b/chromium/third_party/blink/renderer/platform/heap/marking_visitor.cc @@ -39,74 +39,58 @@ MarkingVisitor::MarkingVisitor(ThreadState* state, MarkingMode marking_mode) MarkingVisitor::~MarkingVisitor() = default; -void MarkingVisitor::ConservativelyMarkAddress(BasePage* page, - Address address) { -#if DCHECK_IS_ON() - DCHECK(page->Contains(address)); -#endif +void MarkingVisitor::DynamicallyMarkAddress(Address address) { + BasePage* const page = PageFromObject(address); HeapObjectHeader* const header = page->IsLargeObjectPage() ? static_cast<LargeObjectPage*>(page)->ObjectHeader() : static_cast<NormalPage*>(page)->FindHeaderFromAddress(address); - if (!header) - return; - ConservativelyMarkHeader(header); + DCHECK(header); + DCHECK(!header->IsInConstruction()); + const GCInfo* gc_info = + GCInfoTable::Get().GCInfoFromIndex(header->GcInfoIndex()); + MarkHeader(header, gc_info->trace_); } +void MarkingVisitor::ConservativelyMarkAddress(BasePage* page, + Address address) { #if DCHECK_IS_ON() -void MarkingVisitor::ConservativelyMarkAddress( - BasePage* page, - Address address, - MarkedPointerCallbackForTesting callback) { DCHECK(page->Contains(address)); +#endif HeapObjectHeader* const header = page->IsLargeObjectPage() ? static_cast<LargeObjectPage*>(page)->ObjectHeader() : static_cast<NormalPage*>(page)->FindHeaderFromAddress(address); - if (!header) + if (!header || header->IsMarked()) return; - if (!callback(header)) - ConservativelyMarkHeader(header); -} -#endif // DCHECK_IS_ON - -namespace { - -#if DCHECK_IS_ON() -bool IsUninitializedMemory(void* object_pointer, size_t object_size) { - // Scan through the object's fields and check that they are all zero. - Address* object_fields = reinterpret_cast<Address*>(object_pointer); - for (size_t i = 0; i < object_size / sizeof(Address); ++i) { - if (object_fields[i]) - return false; - } - return true; -} -#endif -} // namespace - -void MarkingVisitor::ConservativelyMarkHeader(HeapObjectHeader* header) { + // Simple case for fully constructed objects. const GCInfo* gc_info = GCInfoTable::Get().GCInfoFromIndex(header->GcInfoIndex()); - if (gc_info->HasVTable() && !VTableInitialized(header->Payload())) { - // We hit this branch when a GC strikes before GarbageCollected<>'s - // constructor runs. - // - // class A : public GarbageCollected<A> { virtual void f() = 0; }; - // class B : public A { - // B() : A(foo()) { }; - // }; - // - // If foo() allocates something and triggers a GC, the vtable of A - // has not yet been initialized. In this case, we should mark the A - // object without tracing any member of the A object. - MarkHeaderNoTracing(header); -#if DCHECK_IS_ON() - DCHECK(IsUninitializedMemory(header->Payload(), header->PayloadSize())); -#endif - } else { + if (!header->IsInConstruction()) { MarkHeader(header, gc_info->trace_); + return; + } + + // This case is reached for not-fully-constructed objects with vtables. + // We can differentiate multiple cases: + // 1. No vtable set up. Example: + // class A : public GarbageCollected<A> { virtual void f() = 0; }; + // class B : public A { B() : A(foo()) {}; }; + // The vtable for A is not set up if foo() allocates and triggers a GC. + // + // 2. Vtables properly set up (non-mixin case). + // 3. Vtables not properly set up (mixin) if GC is allowed during mixin + // construction. + // + // We use a simple conservative approach for these cases as they are not + // performance critical. + MarkHeaderNoTracing(header); + Address* payload = reinterpret_cast<Address*>(header->Payload()); + const size_t payload_size = header->PayloadSize(); + for (size_t i = 0; i < (payload_size / sizeof(Address)); ++i) { + if (payload[i]) + Heap().CheckAndMarkPointer(this, payload[i]); } } diff --git a/chromium/third_party/blink/renderer/platform/heap/marking_visitor.h b/chromium/third_party/blink/renderer/platform/heap/marking_visitor.h index 8892d9cc23b..a53f0bf90a4 100644 --- a/chromium/third_party/blink/renderer/platform/heap/marking_visitor.h +++ b/chromium/third_party/blink/renderer/platform/heap/marking_visitor.h @@ -51,13 +51,15 @@ class PLATFORM_EXPORT MarkingVisitor : public Visitor { // Marking implementation. - // Conservatively marks an object if pointed to by Address. + // Conservatively marks an object if pointed to by Address. The object may + // be in construction as the scan is conservative without relying on a + // Trace method. void ConservativelyMarkAddress(BasePage*, Address); -#if DCHECK_IS_ON() - void ConservativelyMarkAddress(BasePage*, - Address, - MarkedPointerCallbackForTesting); -#endif // DCHECK_IS_ON() + + // Marks an object dynamically using any address within its body and adds a + // tracing callback for processing of the object. The object is not allowed + // to be in construction. + void DynamicallyMarkAddress(Address); // Marks an object and adds a tracing callback for processing of the object. inline void MarkHeader(HeapObjectHeader*, TraceCallback); @@ -98,8 +100,11 @@ class PLATFORM_EXPORT MarkingVisitor : public Visitor { // that lead to many recursions. DCHECK(Heap().GetStackFrameDepth().IsAcceptableStackUse()); if (LIKELY(Heap().GetStackFrameDepth().IsSafeToRecurse())) { - if (MarkHeaderNoTracing( - HeapObjectHeader::FromPayload(desc.base_object_payload))) { + HeapObjectHeader* header = + HeapObjectHeader::FromPayload(desc.base_object_payload); + if (header->IsInConstruction()) { + not_fully_constructed_worklist_.Push(desc.base_object_payload); + } else if (MarkHeaderNoTracing(header)) { desc.callback(this, desc.base_object_payload); } return; @@ -117,6 +122,12 @@ class PLATFORM_EXPORT MarkingVisitor : public Visitor { void** object_slot, TraceDescriptor desc, WeakCallback callback) final { + // Filter out already marked values. The write barrier for WeakMember + // ensures that any newly set value after this point is kept alive and does + // not require the callback. + if (desc.base_object_payload != BlinkGC::kNotFullyConstructedObject && + HeapObjectHeader::FromPayload(desc.base_object_payload)->IsMarked()) + return; RegisterWeakCallback(object_slot, callback); } @@ -170,8 +181,6 @@ class PLATFORM_EXPORT MarkingVisitor : public Visitor { void RegisterBackingStoreReference(void** slot); - void ConservativelyMarkHeader(HeapObjectHeader*); - MarkingWorklist::View marking_worklist_; NotFullyConstructedWorklist::View not_fully_constructed_worklist_; WeakCallbackWorklist::View weak_callback_worklist_; @@ -196,7 +205,9 @@ inline void MarkingVisitor::MarkHeader(HeapObjectHeader* header, DCHECK(header); DCHECK(callback); - if (MarkHeaderNoTracing(header)) { + if (header->IsInConstruction()) { + not_fully_constructed_worklist_.Push(header->Payload()); + } else if (MarkHeaderNoTracing(header)) { marking_worklist_.Push( {reinterpret_cast<void*>(header->Payload()), callback}); } diff --git a/chromium/third_party/blink/renderer/platform/heap/name_trait_test.cc b/chromium/third_party/blink/renderer/platform/heap/name_trait_test.cc index 4e4a43bfb4a..fac0052bd2c 100644 --- a/chromium/third_party/blink/renderer/platform/heap/name_trait_test.cc +++ b/chromium/third_party/blink/renderer/platform/heap/name_trait_test.cc @@ -4,8 +4,8 @@ #include <string.h> +#include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" - #include "third_party/blink/renderer/platform/heap/name_traits.h" namespace blink { @@ -29,10 +29,20 @@ class ClassWithName : public NameClient { } // namespace +TEST(NameTraitTest, InternalNamesHiddenInOfficialBuild) { +#if defined(OFFICIAL_BUILD) + EXPECT_TRUE(NameTrait<ClassWithoutName>::HideInternalName()); +#endif +} + TEST(NameTraitTest, DefaultName) { ClassWithoutName no_name; const char* name = NameTrait<ClassWithoutName>::GetName(&no_name); - EXPECT_EQ(0, strcmp(name, "InternalNode")); + if (NameTrait<ClassWithoutName>::HideInternalName()) { + EXPECT_EQ(0, strcmp(name, "InternalNode")); + } else { + EXPECT_NE(nullptr, strstr(name, "ClassWithoutName")); + } } TEST(NameTraitTest, CustomName) { diff --git a/chromium/third_party/blink/renderer/platform/heap/name_traits.h b/chromium/third_party/blink/renderer/platform/heap/name_traits.h index d1261815876..941fc88b3bc 100644 --- a/chromium/third_party/blink/renderer/platform/heap/name_traits.h +++ b/chromium/third_party/blink/renderer/platform/heap/name_traits.h @@ -5,8 +5,10 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_NAME_TRAITS_H_ #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_NAME_TRAITS_H_ +#include "build/build_config.h" #include "third_party/blink/renderer/platform/bindings/name_client.h" #include "third_party/blink/renderer/platform/wtf/allocator.h" +#include "third_party/blink/renderer/platform/wtf/type_traits.h" namespace blink { @@ -15,6 +17,14 @@ class NameTrait { STATIC_ONLY(NameTrait); public: + static constexpr bool HideInternalName() { +#if defined(OFFICIAL_BUILD) || !(defined(COMPILER_GCC) || defined(__clang__)) + return true; +#else + return false; +#endif + } + static const char* GetName(const void* obj) { return GetNameFor(static_cast<const T*>(obj)); } @@ -24,7 +34,34 @@ class NameTrait { return wrapper_tracable->NameInHeapSnapshot(); } - static const char* GetNameFor(...) { return "InternalNode"; } + static const char* GetNameFor(...) { + // For non-official builds construct the name of a type from a compiler + // intrinsic. + // + // Do not include such type information in official builds to + // (a) safe binary size on string literals, and + // (b) avoid exposing internal types until a proper DevTools frontend + // implementation is present. +#if defined(OFFICIAL_BUILD) || !(defined(COMPILER_GCC) || defined(__clang__)) + return "InternalNode"; +#else + DCHECK(!HideInternalName()); + static const char* leaky_class_name = nullptr; + if (leaky_class_name) + return leaky_class_name; + + // Parsing string of structure: + // const char *WTF::GetStringWithTypeName<TYPE>() [T = TYPE] + // Note that this only works on clang or GCC builds. + const std::string raw(WTF::GetStringWithTypeName<T>()); + const auto start_pos = raw.rfind("T = ") + 4; + DCHECK(std::string::npos != start_pos); + const auto len = raw.length() - start_pos - 1; + const std::string name = raw.substr(start_pos, len).c_str(); + leaky_class_name = strcpy(new char[name.length() + 1], name.c_str()); + return leaky_class_name; +#endif + } }; } // namespace blink diff --git a/chromium/third_party/blink/renderer/platform/heap/page_memory.cc b/chromium/third_party/blink/renderer/platform/heap/page_memory.cc index 2a4d3f2117d..cce3dc9202d 100644 --- a/chromium/third_party/blink/renderer/platform/heap/page_memory.cc +++ b/chromium/third_party/blink/renderer/platform/heap/page_memory.cc @@ -9,7 +9,6 @@ #include "third_party/blink/renderer/platform/heap/heap.h" #include "third_party/blink/renderer/platform/wtf/address_sanitizer.h" #include "third_party/blink/renderer/platform/wtf/assertions.h" -#include "third_party/blink/renderer/platform/wtf/atomics.h" namespace blink { diff --git a/chromium/third_party/blink/renderer/platform/heap/persistent.h b/chromium/third_party/blink/renderer/platform/heap/persistent.h index 52dc689d86f..69db982b4a6 100644 --- a/chromium/third_party/blink/renderer/platform/heap/persistent.h +++ b/chromium/third_party/blink/renderer/platform/heap/persistent.h @@ -13,7 +13,6 @@ #include "third_party/blink/renderer/platform/heap/persistent_node.h" #include "third_party/blink/renderer/platform/heap/visitor.h" #include "third_party/blink/renderer/platform/wtf/allocator.h" -#include "third_party/blink/renderer/platform/wtf/atomics.h" namespace blink { @@ -26,11 +25,6 @@ namespace blink { \ private: -enum WeaknessPersistentConfiguration { - kNonWeakPersistentConfiguration, - kWeakPersistentConfiguration -}; - enum CrossThreadnessPersistentConfiguration { kSingleThreadPersistentConfiguration, kCrossThreadPersistentConfiguration @@ -172,10 +166,9 @@ class PersistentBase { // needing to be cleared out before the thread is terminated. PersistentBase* RegisterAsStaticReference() { CHECK_EQ(weaknessConfiguration, kNonWeakPersistentConfiguration); - if (persistent_node_) { + if (PersistentNode* node = persistent_node_.Get()) { DCHECK(ThreadState::Current()); - ThreadState::Current()->RegisterStaticPersistentNode(persistent_node_, - nullptr); + ThreadState::Current()->RegisterStaticPersistentNode(node, nullptr); LEAK_SANITIZER_IGNORE_OBJECT(this); } return this; @@ -190,11 +183,7 @@ class PersistentBase { ProcessHeap::CrossThreadPersistentMutex().AssertAcquired(); #endif raw_ = nullptr; - CrossThreadPersistentRegion& region = - weaknessConfiguration == kWeakPersistentConfiguration - ? ProcessHeap::GetCrossThreadWeakPersistentRegion() - : ProcessHeap::GetCrossThreadPersistentRegion(); - region.FreePersistentNode(persistent_node_); + persistent_node_.ClearWithLockHeld(); } private: @@ -208,7 +197,7 @@ class PersistentBase { } CheckPointer(); if (raw_) { - if (!persistent_node_) + if (!persistent_node_.IsInitialized()) Initialize(); return; } @@ -229,64 +218,17 @@ class PersistentBase { NO_SANITIZE_ADDRESS void Initialize() { - DCHECK(!persistent_node_); + DCHECK(!persistent_node_.IsInitialized()); if (!raw_ || IsHashTableDeletedValue()) return; TraceCallback trace_callback = TraceMethodDelegate<PersistentBase, &PersistentBase::TracePersistent>::Trampoline; - if (crossThreadnessConfiguration == kCrossThreadPersistentConfiguration) { - CrossThreadPersistentRegion& region = - weaknessConfiguration == kWeakPersistentConfiguration - ? ProcessHeap::GetCrossThreadWeakPersistentRegion() - : ProcessHeap::GetCrossThreadPersistentRegion(); - MutexLocker lock(ProcessHeap::CrossThreadPersistentMutex()); - region.AllocatePersistentNode(persistent_node_, this, trace_callback); - return; - } - ThreadState* state = - ThreadStateFor<ThreadingTrait<T>::kAffinity>::GetState(); - DCHECK(state->CheckThread()); - PersistentRegion* region = - weaknessConfiguration == kWeakPersistentConfiguration - ? state->GetWeakPersistentRegion() - : state->GetPersistentRegion(); - persistent_node_ = region->AllocatePersistentNode(this, trace_callback); -#if DCHECK_IS_ON() - state_ = state; -#endif + persistent_node_.Initialize(this, trace_callback); } - void Uninitialize() { - if (crossThreadnessConfiguration == kCrossThreadPersistentConfiguration) { - if (AcquireLoad(reinterpret_cast<void* volatile*>(&persistent_node_))) { - CrossThreadPersistentRegion& region = - weaknessConfiguration == kWeakPersistentConfiguration - ? ProcessHeap::GetCrossThreadWeakPersistentRegion() - : ProcessHeap::GetCrossThreadPersistentRegion(); - MutexLocker lock(ProcessHeap::CrossThreadPersistentMutex()); - region.FreePersistentNode(persistent_node_); - } - return; - } - - if (!persistent_node_) - return; - ThreadState* state = - ThreadStateFor<ThreadingTrait<T>::kAffinity>::GetState(); - DCHECK(state->CheckThread()); - // Persistent handle must be created and destructed in the same thread. -#if DCHECK_IS_ON() - DCHECK_EQ(state_, state); -#endif - PersistentRegion* region = - weaknessConfiguration == kWeakPersistentConfiguration - ? state->GetWeakPersistentRegion() - : state->GetPersistentRegion(); - state->FreePersistentNode(region, persistent_node_); - persistent_node_ = nullptr; - } + void Uninitialize() { persistent_node_.Uninitialize(); } void CheckPointer() const { #if DCHECK_IS_ON() @@ -327,7 +269,7 @@ class PersistentBase { weaknessConfiguration, crossThreadnessConfiguration>; Base* persistent = reinterpret_cast<Base*>(persistent_pointer); T* object = persistent->Get(); - if (object && !ObjectAliveTrait<T>::IsHeapObjectAlive(object)) + if (object && !ThreadHeap::IsHeapObjectAlive(object)) ClearWeakPersistent(persistent); } @@ -353,11 +295,22 @@ class PersistentBase { NOTREACHED(); } - // m_raw is accessed most, so put it at the first field. + // raw_ is accessed most, so put it at the first field. T* raw_; - PersistentNode* persistent_node_ = nullptr; + + // The pointer to the underlying persistent node. + // + // Since accesses are atomics in the cross-thread case, a different type is + // needed to prevent the compiler producing an error when it encounters + // operations that are legal on raw pointers but not on atomics, or + // vice-versa. + std::conditional_t< + crossThreadnessConfiguration == kCrossThreadPersistentConfiguration, + CrossThreadPersistentNodePtr<weaknessConfiguration>, + PersistentNodePtr<ThreadingTrait<T>::kAffinity, weaknessConfiguration>> + persistent_node_; + #if DCHECK_IS_ON() - ThreadState* state_ = nullptr; const ThreadState* creation_thread_state_; #endif }; diff --git a/chromium/third_party/blink/renderer/platform/heap/persistent_node.h b/chromium/third_party/blink/renderer/platform/heap/persistent_node.h index 596ce270b33..31c31cd6f66 100644 --- a/chromium/third_party/blink/renderer/platform/heap/persistent_node.h +++ b/chromium/third_party/blink/renderer/platform/heap/persistent_node.h @@ -5,18 +5,24 @@ #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_PERSISTENT_NODE_H_ #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_PERSISTENT_NODE_H_ +#include <atomic> #include <memory> #include "third_party/blink/renderer/platform/heap/process_heap.h" #include "third_party/blink/renderer/platform/heap/thread_state.h" #include "third_party/blink/renderer/platform/platform_export.h" #include "third_party/blink/renderer/platform/wtf/allocator.h" #include "third_party/blink/renderer/platform/wtf/assertions.h" -#include "third_party/blink/renderer/platform/wtf/atomics.h" #include "third_party/blink/renderer/platform/wtf/threading_primitives.h" namespace blink { class CrossThreadPersistentRegion; +class PersistentRegion; + +enum WeaknessPersistentConfiguration { + kNonWeakPersistentConfiguration, + kWeakPersistentConfiguration +}; class PersistentNode final { DISALLOW_NEW(); @@ -97,6 +103,52 @@ struct PersistentNodeSlots final { friend class CrossThreadPersistentRegion; }; +// Used by PersistentBase to manage a pointer to a thread heap persistent node. +// This class mostly passes accesses through, but provides an interface +// compatible with CrossThreadPersistentNodePtr. +template <ThreadAffinity affinity, + WeaknessPersistentConfiguration weakness_configuration> +class PersistentNodePtr { + public: + PersistentNode* Get() const { return ptr_; } + bool IsInitialized() const { return ptr_; } + + void Initialize(void* owner, TraceCallback); + void Uninitialize(); + + private: + PersistentNode* ptr_ = nullptr; +#if DCHECK_IS_ON() + ThreadState* state_ = nullptr; +#endif +}; + +// Used by PersistentBase to manage a pointer to a cross-thread persistent node. +// It uses ProcessHeap::CrossThreadPersistentMutex() to protect most accesses, +// but can be polled to see whether it is initialized without the mutex. +template <WeaknessPersistentConfiguration weakness_configuration> +class CrossThreadPersistentNodePtr { + public: + PersistentNode* Get() const { +#if DCHECK_IS_ON() + ProcessHeap::CrossThreadPersistentMutex().AssertAcquired(); +#endif + return ptr_.load(std::memory_order_relaxed); + } + bool IsInitialized() const { return ptr_.load(std::memory_order_acquire); } + + void Initialize(void* owner, TraceCallback); + void Uninitialize(); + + void ClearWithLockHeld(); + + private: + // Access must either be protected by the cross-thread persistent mutex or + // handle the fact that this may be changed concurrently (with a + // release-store). + std::atomic<PersistentNode*> ptr_{nullptr}; +}; + // PersistentRegion provides a region of PersistentNodes. PersistentRegion // holds a linked list of PersistentNodeSlots, each of which stores // a predefined number of PersistentNodes. You can call allocatePersistentNode/ @@ -171,18 +223,14 @@ class CrossThreadPersistentRegion final { USING_FAST_MALLOC(CrossThreadPersistentRegion); public: - void AllocatePersistentNode(PersistentNode*& persistent_node, - void* self, - TraceCallback trace) { + PersistentNode* AllocatePersistentNode(void* self, TraceCallback trace) { #if DCHECK_IS_ON() ProcessHeap::CrossThreadPersistentMutex().AssertAcquired(); #endif - PersistentNode* node = - persistent_region_.AllocatePersistentNode(self, trace); - ReleaseStore(reinterpret_cast<void* volatile*>(&persistent_node), node); + return persistent_region_.AllocatePersistentNode(self, trace); } - void FreePersistentNode(PersistentNode*& persistent_node) { + void FreePersistentNode(PersistentNode* node) { #if DCHECK_IS_ON() ProcessHeap::CrossThreadPersistentMutex().AssertAcquired(); #endif @@ -196,10 +244,9 @@ class CrossThreadPersistentRegion final { // The lock ensures the updating is ordered, but by the time lock has been // acquired the PersistentNode reference may have been cleared out already; // check for this. - if (!persistent_node) + if (!node) return; - persistent_region_.FreePersistentNode(persistent_node); - ReleaseStore(reinterpret_cast<void* volatile*>(&persistent_node), nullptr); + persistent_region_.FreePersistentNode(node); } void TracePersistentNodes(Visitor* visitor) { @@ -228,6 +275,92 @@ class CrossThreadPersistentRegion final { PersistentRegion persistent_region_; }; +template <ThreadAffinity affinity, + WeaknessPersistentConfiguration weakness_configuration> +void PersistentNodePtr<affinity, weakness_configuration>::Initialize( + void* owner, + TraceCallback trace_callback) { + ThreadState* state = ThreadStateFor<affinity>::GetState(); + DCHECK(state->CheckThread()); + PersistentRegion* region = + weakness_configuration == kWeakPersistentConfiguration + ? state->GetWeakPersistentRegion() + : state->GetPersistentRegion(); + ptr_ = region->AllocatePersistentNode(owner, trace_callback); +#if DCHECK_IS_ON() + state_ = state; +#endif +} + +template <ThreadAffinity affinity, + WeaknessPersistentConfiguration weakness_configuration> +void PersistentNodePtr<affinity, weakness_configuration>::Uninitialize() { + if (!ptr_) + return; + ThreadState* state = ThreadStateFor<affinity>::GetState(); + DCHECK(state->CheckThread()); +#if DCHECK_IS_ON() + DCHECK_EQ(state_, state) + << "must be initialized and uninitialized on the same thread"; + state_ = nullptr; +#endif + PersistentRegion* region = + weakness_configuration == kWeakPersistentConfiguration + ? state->GetWeakPersistentRegion() + : state->GetPersistentRegion(); + state->FreePersistentNode(region, ptr_); + ptr_ = nullptr; +} + +template <WeaknessPersistentConfiguration weakness_configuration> +void CrossThreadPersistentNodePtr<weakness_configuration>::Initialize( + void* owner, + TraceCallback trace_callback) { + CrossThreadPersistentRegion& region = + weakness_configuration == kWeakPersistentConfiguration + ? ProcessHeap::GetCrossThreadWeakPersistentRegion() + : ProcessHeap::GetCrossThreadPersistentRegion(); + MutexLocker lock(ProcessHeap::CrossThreadPersistentMutex()); + PersistentNode* node = region.AllocatePersistentNode(owner, trace_callback); + ptr_.store(node, std::memory_order_release); +} + +template <WeaknessPersistentConfiguration weakness_configuration> +void CrossThreadPersistentNodePtr<weakness_configuration>::Uninitialize() { + // As an optimization, skip the mutex acquisition. + // + // Persistent handles are often assigned or destroyed while being + // uninitialized. + // + // Calling code is still expected to synchronize mutations to persistent + // handles, so if this thread can see the node pointer as having been + // cleared and the program does not have a data race, then this pointer would + // still have been blank after waiting for the cross-thread persistent mutex. + if (!ptr_.load(std::memory_order_acquire)) + return; + + CrossThreadPersistentRegion& region = + weakness_configuration == kWeakPersistentConfiguration + ? ProcessHeap::GetCrossThreadWeakPersistentRegion() + : ProcessHeap::GetCrossThreadPersistentRegion(); + MutexLocker lock(ProcessHeap::CrossThreadPersistentMutex()); + region.FreePersistentNode(ptr_.load(std::memory_order_relaxed)); + ptr_.store(nullptr, std::memory_order_release); +} + +template <WeaknessPersistentConfiguration weakness_configuration> +void CrossThreadPersistentNodePtr<weakness_configuration>::ClearWithLockHeld() { +#if DCHECK_IS_ON() + ProcessHeap::CrossThreadPersistentMutex().AssertAcquired(); +#endif + CrossThreadPersistentRegion& region = + weakness_configuration == kWeakPersistentConfiguration + ? ProcessHeap::GetCrossThreadWeakPersistentRegion() + : ProcessHeap::GetCrossThreadPersistentRegion(); + region.FreePersistentNode(ptr_.load(std::memory_order_relaxed)); + ptr_.store(nullptr, std::memory_order_release); +} + } // namespace blink #endif diff --git a/chromium/third_party/blink/renderer/platform/heap/persistent_test.cc b/chromium/third_party/blink/renderer/platform/heap/persistent_test.cc index fad42771309..11d0dd51e57 100644 --- a/chromium/third_party/blink/renderer/platform/heap/persistent_test.cc +++ b/chromium/third_party/blink/renderer/platform/heap/persistent_test.cc @@ -22,7 +22,7 @@ class Receiver : public GarbageCollected<Receiver> { }; TEST(PersistentTest, BindCancellation) { - Receiver* receiver = new Receiver; + Receiver* receiver = MakeGarbageCollected<Receiver>(); int counter = 0; base::RepeatingClosure function = WTF::BindRepeating(&Receiver::Increment, WrapWeakPersistent(receiver), @@ -38,7 +38,7 @@ TEST(PersistentTest, BindCancellation) { } TEST(PersistentTest, CrossThreadBindCancellation) { - Receiver* receiver = new Receiver; + Receiver* receiver = MakeGarbageCollected<Receiver>(); int counter = 0; CrossThreadClosure function = blink::CrossThreadBind( &Receiver::Increment, WrapCrossThreadWeakPersistent(receiver), diff --git a/chromium/third_party/blink/renderer/platform/heap/run_all_tests.cc b/chromium/third_party/blink/renderer/platform/heap/run_all_tests.cc index d8ee785ad2d..ea68f22d0c2 100644 --- a/chromium/third_party/blink/renderer/platform/heap/run_all_tests.cc +++ b/chromium/third_party/blink/renderer/platform/heap/run_all_tests.cc @@ -35,24 +35,29 @@ #include "content/public/test/blink_test_environment.h" #include "third_party/blink/renderer/platform/heap/thread_state.h" -class BlinkTestEnvironmentScope { +class BlinkTestSuite : public base::TestSuite { public: - BlinkTestEnvironmentScope() { content::SetUpBlinkTestEnvironment(); } - ~BlinkTestEnvironmentScope() { content::TearDownBlinkTestEnvironment(); } -}; + BlinkTestSuite(int argc, char** argv) : base::TestSuite(argc, argv) {} -int runHelper(base::TestSuite* testSuite) { - BlinkTestEnvironmentScope blinkTestEnvironment; - blink::ThreadState* currentThreadState = blink::ThreadState::Current(); - currentThreadState->RegisterTraceDOMWrappers(nullptr, nullptr, nullptr, - nullptr); - int result = testSuite->Run(); - currentThreadState->CollectAllGarbage(); - return result; -} + private: + void Initialize() override { + base::TestSuite::Initialize(); + content::SetUpBlinkTestEnvironment(); + blink::ThreadState::Current()->RegisterTraceDOMWrappers(nullptr, nullptr, + nullptr, nullptr); + } + void Shutdown() override { + blink::ThreadState::Current()->CollectAllGarbage(); + content::TearDownBlinkTestEnvironment(); + base::TestSuite::Shutdown(); + } + + DISALLOW_COPY_AND_ASSIGN(BlinkTestSuite); +}; int main(int argc, char** argv) { - base::TestSuite testSuite(argc, argv); + BlinkTestSuite testSuite(argc, argv); return base::LaunchUnitTests( - argc, argv, base::BindOnce(runHelper, base::Unretained(&testSuite))); + argc, argv, + base::BindOnce(&base::TestSuite::Run, base::Unretained(&testSuite))); } diff --git a/chromium/third_party/blink/renderer/platform/heap/thread_state.cc b/chromium/third_party/blink/renderer/platform/heap/thread_state.cc index 198af3f0761..a35433abd8c 100644 --- a/chromium/third_party/blink/renderer/platform/heap/thread_state.cc +++ b/chromium/third_party/blink/renderer/platform/heap/thread_state.cc @@ -177,7 +177,6 @@ ThreadState::ThreadState() mixins_being_constructed_count_(0), object_resurrection_forbidden_(false), in_atomic_pause_(false), - gc_mixin_marker_(nullptr), gc_state_(kNoGCScheduled), gc_phase_(GCPhase::kNone), reason_for_scheduled_gc_(BlinkGC::GCReason::kMaxValue), @@ -511,9 +510,14 @@ void ThreadState::ScheduleV8FollowupGCIfNeeded(BlinkGC::V8GCType gc_type) { if (IsGCForbidden()) return; - // This completeSweep() will do nothing in common cases since we've - // called completeSweep() before V8 starts minor/major GCs. if (gc_type == BlinkGC::kV8MajorGC) { + // In case of unified heap garbage collections a V8 major GC also collects + // the Blink heap. + if (RuntimeEnabledFeatures::HeapUnifiedGarbageCollectionEnabled()) + return; + + // This CompleteSweep() will do nothing in common cases since we've + // called CompleteSweep() before V8 starts minor/major GCs. // TODO(ulan): Try removing this for Major V8 GC too. CompleteSweep(); DCHECK(!IsSweepingInProgress()); @@ -535,13 +539,10 @@ void ThreadState::ScheduleV8FollowupGCIfNeeded(BlinkGC::V8GCType gc_type) { << "ScheduleV8FollowupGCIfNeeded: Scheduled precise GC"; SchedulePreciseGC(); } - return; - } - if (gc_type == BlinkGC::kV8MajorGC && ShouldScheduleIdleGC()) { + } else if (gc_type == BlinkGC::kV8MajorGC && ShouldScheduleIdleGC()) { VLOG(2) << "[state:" << this << "] " << "ScheduleV8FollowupGCIfNeeded: Scheduled idle GC"; ScheduleIdleGC(); - return; } } @@ -930,7 +931,7 @@ void ThreadState::RunScheduledGC(BlinkGC::StackState stack_state) { // Idle time GC will be scheduled by Blink Scheduler. break; case kIncrementalMarkingStepScheduled: - IncrementalMarkingStep(); + IncrementalMarkingStep(stack_state); break; case kIncrementalMarkingFinalizeScheduled: IncrementalMarkingFinalize(); @@ -1400,32 +1401,32 @@ void ThreadState::InvokePreFinalizers() { } // static -base::subtle::AtomicWord ThreadState::incremental_marking_counter_ = 0; +AtomicEntryFlag ThreadState::incremental_marking_flag_; // static -base::subtle::AtomicWord ThreadState::wrapper_tracing_counter_ = 0; +AtomicEntryFlag ThreadState::wrapper_tracing_flag_; void ThreadState::EnableIncrementalMarkingBarrier() { CHECK(!IsIncrementalMarking()); - base::subtle::Barrier_AtomicIncrement(&incremental_marking_counter_, 1); + incremental_marking_flag_.Enter(); SetIncrementalMarking(true); } void ThreadState::DisableIncrementalMarkingBarrier() { CHECK(IsIncrementalMarking()); - base::subtle::Barrier_AtomicIncrement(&incremental_marking_counter_, -1); + incremental_marking_flag_.Exit(); SetIncrementalMarking(false); } void ThreadState::EnableWrapperTracingBarrier() { CHECK(!IsWrapperTracing()); - base::subtle::Barrier_AtomicIncrement(&wrapper_tracing_counter_, 1); + wrapper_tracing_flag_.Enter(); SetWrapperTracing(true); } void ThreadState::DisableWrapperTracingBarrier() { CHECK(IsWrapperTracing()); - base::subtle::Barrier_AtomicIncrement(&wrapper_tracing_counter_, -1); + wrapper_tracing_flag_.Exit(); SetWrapperTracing(false); } @@ -1453,7 +1454,7 @@ void ThreadState::IncrementalMarkingStart(BlinkGC::GCReason reason) { } } -void ThreadState::IncrementalMarkingStep() { +void ThreadState::IncrementalMarkingStep(BlinkGC::StackState stack_state) { DCHECK(IsMarkingInProgress()); ThreadHeapStatsCollector::EnabledScope stats_scope( @@ -1463,6 +1464,9 @@ void ThreadState::IncrementalMarkingStep() { << "IncrementalMarking: Step " << "Reason: " << GcReasonString(current_gc_data_.reason); AtomicPauseScope atomic_pause_scope(this); + if (stack_state == BlinkGC::kNoHeapPointersOnStack) { + Heap().FlushNotFullyConstructedObjects(); + } const bool complete = MarkPhaseAdvanceMarking( CurrentTimeTicks() + next_incremental_marking_step_duration_); if (complete) { @@ -1695,6 +1699,11 @@ void ThreadState::MarkPhasePrologue(BlinkGC::StackState stack_state, void ThreadState::AtomicPausePrologue(BlinkGC::StackState stack_state, BlinkGC::MarkingType marking_type, BlinkGC::GCReason reason) { + // Compaction needs to be canceled when incremental marking ends with a + // conservative GC. + if (stack_state == BlinkGC::kHeapPointersOnStack) + Heap().Compaction()->CancelCompaction(); + if (IsMarkingInProgress()) { // Incremental marking is already in progress. Only update the state // that is necessary to update. @@ -1713,6 +1722,10 @@ void ThreadState::AtomicPausePrologue(BlinkGC::StackState stack_state, if (isolate_ && perform_cleanup_) perform_cleanup_(isolate_); + if (stack_state == BlinkGC::kNoHeapPointersOnStack) { + Heap().FlushNotFullyConstructedObjects(); + } + DCHECK(InAtomicMarkingPause()); Heap().MakeConsistentForGC(); Heap().ClearArenaAges(); diff --git a/chromium/third_party/blink/renderer/platform/heap/thread_state.h b/chromium/third_party/blink/renderer/platform/heap/thread_state.h index d04c2b07051..00282d672c4 100644 --- a/chromium/third_party/blink/renderer/platform/heap/thread_state.h +++ b/chromium/third_party/blink/renderer/platform/heap/thread_state.h @@ -33,10 +33,10 @@ #include <memory> -#include "base/atomicops.h" #include "base/macros.h" #include "third_party/blink/public/platform/scheduler/web_rail_mode_observer.h" #include "third_party/blink/renderer/platform/bindings/script_forbidden_scope.h" +#include "third_party/blink/renderer/platform/heap/atomic_entry_flag.h" #include "third_party/blink/renderer/platform/heap/blink_gc.h" #include "third_party/blink/renderer/platform/heap/threading_traits.h" #include "third_party/blink/renderer/platform/platform_export.h" @@ -62,7 +62,6 @@ class IncrementalMarkingScope; class IncrementalMarkingTestDriver; } // namespace incremental_marking_test -class GarbageCollectedMixinConstructorMarkerBase; class MarkingVisitor; class PersistentNode; class PersistentRegion; @@ -219,19 +218,23 @@ class PLATFORM_EXPORT ThreadState final ThreadState* state_; }; - // Returns true if any thread is currently incremental marking its heap and - // false otherwise. For an exact check use - // ThreadState::IsIncrementalMarking(). + // Returns true if some thread (possibly the current thread) may be doing + // incremental marking. If false is returned, the *current* thread is + // definitely not doing incremental marking. See atomic_entry_flag.h for + // details. + // + // For an exact check, use ThreadState::IsIncrementalMarking. ALWAYS_INLINE static bool IsAnyIncrementalMarking() { - // Stores use full barrier to allow using the simplest relaxed load here. - return base::subtle::NoBarrier_Load(&incremental_marking_counter_) > 0; + return incremental_marking_flag_.MightBeEntered(); } - // Returns true if any thread is currently incremental marking its heap and - // false otherwise. For an exact check use ThreadState::IsWrapperTracing(). + // Returns true if some thread (possibly the current thread) may be doing + // wrapper tracing. If false is returned, the *current* thread is definitely + // not doing wrapper tracing. See atomic_entry_flag.h for details. + // + // For an exact check, use ThreadState::IsWrapperTracing. static bool IsAnyWrapperTracing() { - // Stores use full barrier to allow using the simplest relaxed load here. - return base::subtle::NoBarrier_Load(&wrapper_tracing_counter_) > 0; + return wrapper_tracing_flag_.MightBeEntered(); } static void AttachMainThread(); @@ -300,7 +303,7 @@ class PLATFORM_EXPORT ThreadState final void ScheduleIncrementalMarkingFinalize(); void IncrementalMarkingStart(BlinkGC::GCReason); - void IncrementalMarkingStep(); + void IncrementalMarkingStep(BlinkGC::StackState); void IncrementalMarkingFinalize(); bool FinishIncrementalMarkingIfRunning(BlinkGC::StackState, BlinkGC::MarkingType, @@ -476,29 +479,6 @@ class PLATFORM_EXPORT ThreadState final perform_cleanup_ = perform_cleanup; } - // By entering a gc-forbidden scope, conservative GCs will not - // be allowed while handling an out-of-line allocation request. - // Intended used when constructing subclasses of GC mixins, where - // the object being constructed cannot be safely traced & marked - // fully should a GC be allowed while its subclasses are being - // constructed. - void EnterGCForbiddenScopeIfNeeded( - GarbageCollectedMixinConstructorMarkerBase* gc_mixin_marker) { - DCHECK(CheckThread()); - if (!gc_mixin_marker_) { - EnterMixinConstructionScope(); - gc_mixin_marker_ = gc_mixin_marker; - } - } - void LeaveGCForbiddenScopeIfNeeded( - GarbageCollectedMixinConstructorMarkerBase* gc_mixin_marker) { - DCHECK(CheckThread()); - if (gc_mixin_marker_ == gc_mixin_marker) { - LeaveMixinConstructionScope(); - gc_mixin_marker_ = nullptr; - } - } - void FreePersistentNode(PersistentRegion*, PersistentNode*); using PersistentClearCallback = void (*)(void*); @@ -569,13 +549,11 @@ class PLATFORM_EXPORT ThreadState final } private: - // Number of ThreadState's that are currently in incremental marking. The - // counter is incremented by one when some ThreadState enters incremental - // marking and decremented upon finishing. - static base::subtle::AtomicWord incremental_marking_counter_; + // Stores whether some ThreadState is currently in incremental marking. + static AtomicEntryFlag incremental_marking_flag_; - // Same semantic as |incremental_marking_counter_|. - static base::subtle::AtomicWord wrapper_tracing_counter_; + // Same semantic as |incremental_marking_flag_|. + static AtomicEntryFlag wrapper_tracing_flag_; ThreadState(); ~ThreadState() override; @@ -705,8 +683,6 @@ class PLATFORM_EXPORT ThreadState final TimeDelta next_incremental_marking_step_duration_; TimeDelta previous_incremental_marking_time_left_; - GarbageCollectedMixinConstructorMarkerBase* gc_mixin_marker_; - GCState gc_state_; GCPhase gc_phase_; BlinkGC::GCReason reason_for_scheduled_gc_; diff --git a/chromium/third_party/blink/renderer/platform/heap/worklist.h b/chromium/third_party/blink/renderer/platform/heap/worklist.h index c5d96b22fed..fb5c5dd83b5 100644 --- a/chromium/third_party/blink/renderer/platform/heap/worklist.h +++ b/chromium/third_party/blink/renderer/platform/heap/worklist.h @@ -30,12 +30,14 @@ namespace blink { // // Work stealing is best effort, i.e., there is no way to inform other tasks // of the need of items. -template <typename EntryType, int segment_size, int max_tasks = 1> +template <typename _EntryType, int segment_size, int max_tasks = 1> class Worklist { USING_FAST_MALLOC(Worklist); - using WorklistType = Worklist<EntryType, segment_size, max_tasks>; + using WorklistType = Worklist<_EntryType, segment_size, max_tasks>; public: + using EntryType = _EntryType; + class View { public: View(WorklistType* worklist, int task_id) |