[oilpan]: Make thread shutdown more robust.

Source/platform/heap/Heap.h

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 51 matching lines @@
 // Double precision floats are more efficient when 8 byte aligned, so we 8 byte
 // align all allocations even on 32 bit.
 const size_t allocationGranularity = 8;
 const size_t allocationMask = allocationGranularity - 1;
 const size_t objectStartBitMapSize = (blinkPageSize + ((8 * allocationGranularity) - 1)) / (8 * allocationGranularity);
 const size_t reservedForObjectBitMap = ((objectStartBitMapSize + allocationMask) & ~allocationMask);
 const size_t maxHeapObjectSize = 1 << 27;
 
 const size_t markBitMask = 1;
 const size_t freeListMask = 2;
-const size_t debugBitMask = 4;
+// The dead bit is used for objects that have gone through a GC marking, but did
+// not get swept before a new GC started. In that case we set the dead bit on
+// objects that were not marked in the previous GC to ensure we are not reviving

[Mads Ager (chromium), 2014/07/08 08:24:56]

I think we should avoid using the word 'reviving' here (the object doesn't
actually become live again).

... to ensure that we are not tracing them via a conservatively found pointer.
Tracing dead objects could lead to tracing of already finalized objects in
another thread's heap which is a use-after-free situation.

?

[wibling-chromium, 2014/07/08 13:39:47]

Done.

+// them via a conservatively found pointer. This guarantees that we don't end up
+// tracing a revived object pointing into another thread heap's object where the
+// other thread did a sweep and cleared out the object.
+const size_t deadBitMask = 4;
 const size_t sizeMask = ~7;
 const uint8_t freelistZapValue = 42;
 const uint8_t finalizedZapValue = 24;
 
 class HeapStats;
 class PageMemory;
 template<ThreadAffinity affinity> class ThreadLocalPersistents;
 template<typename T, typename RootsAccessor = ThreadLocalPersistents<ThreadingTrait<T>::Affinity > > class Persistent;
 template<typename T> class CrossThreadPersistent;
 
@@ skipping 37 matching lines @@
 
 // Sanity check for a page header address: the address of the page
 // header should be OS page size away from being Blink page size
 // aligned.
 inline bool isPageHeaderAddress(Address address)
 {
     return !((reinterpret_cast<uintptr_t>(address) & blinkPageOffsetMask) - osPageSize());
 }
 #endif
 
-// Mask an address down to the enclosing oilpan heap page base address.
+// Mask an address down to the enclosing oilpan heap base page.
 // All oilpan heap pages are aligned at blinkPageBase plus an OS page size.
 // FIXME: Remove PLATFORM_EXPORT once we get a proper public interface to our typed heaps.
 // This is only exported to enable tests in HeapTest.cpp.
-PLATFORM_EXPORT inline Address pageHeaderAddress(Address address)
+PLATFORM_EXPORT inline BaseHeapPage* pageHeaderFromObject(const void* object)
 {
-    return blinkPageAddress(address) + osPageSize();
+    Address address = reinterpret_cast<Address>(const_cast<void*>(object));
+    return reinterpret_cast<BaseHeapPage*>(blinkPageAddress(address) + osPageSize());
 }
 
-// Common header for heap pages.
-class BaseHeapPage {
-public:
-    BaseHeapPage(PageMemory* storage, const GCInfo* gcInfo, ThreadState* state)
-        : m_storage(storage)
-        , m_gcInfo(gcInfo)
-        , m_threadState(state)
-        , m_padding(0)
-    {
-        ASSERT(isPageHeaderAddress(reinterpret_cast<Address>(this)));
-    }
-
-    // Check if the given address points to an object in this
-    // heap page. If so, find the start of that object and mark it
-    // using the given Visitor. Otherwise do nothing. The pointer must
-    // be within the same aligned blinkPageSize as the this-pointer.
-    //
-    // This is used during conservative stack scanning to
-    // conservatively mark all objects that could be referenced from
-    // the stack.
-    virtual void checkAndMarkPointer(Visitor*, Address) = 0;
-
-#if ENABLE(GC_TRACING)
-    virtual const GCInfo* findGCInfo(Address) = 0;
-#endif
-
-    Address address() { return reinterpret_cast<Address>(this); }
-    PageMemory* storage() const { return m_storage; }
-    ThreadState* threadState() const { return m_threadState; }
-    const GCInfo* gcInfo() { return m_gcInfo; }
-    virtual bool isLargeObject() { return false; }
-
-private:
-    // Accessor to silence unused warnings for the m_padding field.
-    intptr_t padding() const { return m_padding; }
-
-    PageMemory* m_storage;
-    const GCInfo* m_gcInfo;
-    ThreadState* m_threadState;
-    // Pointer sized integer to ensure proper alignment of the
-    // HeapPage header. This can be used as a bit field if we need
-    // to associate more information with pages.
-    intptr_t m_padding;
-};
-
 // Large allocations are allocated as separate objects and linked in a
 // list.
 //
 // 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.
 //
 // The layout of a large heap object is as follows:
 //
 // | BaseHeapPage | next pointer | FinalizedHeapObjectHeader or HeapObjectHeader | payload |
@@ skipping 31 matching lines @@
     // The LargeHeapObject pseudo-page contains one actual object. Determine
     // whether the pointer is within that object.
     bool objectContains(Address object)
     {
         return (payload() <= object) && (object < address() + size());
     }
 
     // Returns true for any address that is on one of the pages that this
     // large object uses. That ensures that we can use a negative result to
     // populate the negative page cache.
-    bool contains(Address object)
+    virtual bool contains(Address object) OVERRIDE
     {
         return roundToBlinkPageStart(address()) <= object && object < roundToBlinkPageEnd(address() + size());
     }
 
     LargeHeapObject<Header>* next()
     {
         return m_next;
     }
 
     size_t size()
     {
         return heapObjectHeader()->size() + sizeof(LargeHeapObject<Header>) + headerPadding<Header>();
     }
 
     Address payload() { return heapObjectHeader()->payload(); }
     size_t payloadSize() { return heapObjectHeader()->payloadSize(); }
 
     Header* heapObjectHeader()
     {
         Address headerAddress = address() + sizeof(LargeHeapObject<Header>) + headerPadding<Header>();
         return reinterpret_cast<Header*>(headerAddress);
     }
 
     bool isMarked();
     void unmark();
     void getStats(HeapStats&);
     void mark(Visitor*);
     void finalize();
+    void setDeadMark();
+    virtual void markOrphaned()
+    {
+        // We clear out the payload to detect incorrect usage.

[haraken, 2014/07/08 05:44:51]

Don't we want to zap the orphaned page in Debug builds?

[wibling-chromium, 2014/07/08 13:39:46]

Good idea. I added a new zap value to detect orphaned pages. I did it in both
Release and Debug since it should perform the same AFAICT.

+        memset(payload(), 0, payloadSize());
+        clearReuseMemory(); // Don't reuse memory for large objects, ie. don't move to the memory pool.

[Mads Ager (chromium), 2014/07/08 08:24:56]

It looks like this is only used to indicate that this page is a large object
page. However, we already know that because we have a virtual isLargeObject
method. Can we use that and remove the duplication of that information in the
reuse memory flag?

[wibling-chromium, 2014/07/08 13:39:47]

Good idea. Done.

+        BaseHeapPage::markOrphaned();
+    }
 
 private:
     friend class ThreadHeap<Header>;
 
     LargeHeapObject<Header>* m_next;
 };
 
 // The BasicObjectHeader is the minimal object header. It is used when
 // encountering heap space of size allocationGranularity to mark it as
 // as freelist entry.
@@ skipping 48 matching lines @@
 
     inline void mark();
     inline void unmark();
 
     inline const GCInfo* gcInfo() { return 0; }
 
     inline Address payload();
     inline size_t payloadSize();
     inline Address payloadEnd();
 
-    inline void setDebugMark();
-    inline void clearDebugMark();
-    inline bool hasDebugMark() const;
+    inline void setDeadMark();
+    inline void clearDeadMark();
+    inline bool hasDeadMark() const;
 
     // Zap magic number with a new magic number that means there was once an
     // object allocated here, but it was freed because nobody marked it during
     // GC.
     void zapMagic();
 
     static void finalize(const GCInfo*, Address, size_t);
     static HeapObjectHeader* fromPayload(const void*);
 
     static const intptr_t magic = 0xc0de247;
@@ skipping 116 matching lines @@
     HeapPage(PageMemory*, ThreadHeap<Header>*, const GCInfo*);
 
     void link(HeapPage**);
     static void unlink(HeapPage*, HeapPage**);
 
     bool isEmpty();
 
     // Returns true for the whole blinkPageSize page that the page is on, even
     // for the header, and the unmapped guard page at the start. That ensures
     // the result can be used to populate the negative page cache.
-    bool contains(Address addr)
+    virtual bool contains(Address addr) OVERRIDE
     {
         Address blinkPageStart = roundToBlinkPageStart(address());
         ASSERT(blinkPageStart == address() - osPageSize()); // Page is at aligned address plus guard page size.
         return blinkPageStart <= addr && addr < blinkPageStart + blinkPageSize;
     }
 
     HeapPage* next() { return m_next; }
 
     Address payload()
     {
         return address() + sizeof(*this) + headerPadding<Header>();
     }
 
     static size_t payloadSize()
     {
         return (blinkPagePayloadSize() - sizeof(HeapPage) - headerPadding<Header>()) & ~allocationMask;
     }
 
     Address end() { return payload() + payloadSize(); }
 
     void getStats(HeapStats&);
-    void clearMarks();
+    void clearLiveAndMarkDead();
     void sweep();
     void clearObjectStartBitMap();
     void finalize(Header*);
     virtual void checkAndMarkPointer(Visitor*, Address) OVERRIDE;
 #if ENABLE(GC_TRACING)
     const GCInfo* findGCInfo(Address) OVERRIDE;
 #endif
     ThreadHeap<Header>* heap() { return m_heap; }
 #if defined(ADDRESS_SANITIZER)
     void poisonUnmarkedObjects();
 #endif
+    virtual void markOrphaned()
+    {
+        // We clear out the payload to detect incorrect usage.

[Mads Ager (chromium), 2014/07/08 08:24:56]

Maybe update the comment to state that incorrect usage means cross-thread
pointers into the heap of a dead thread?

[wibling-chromium, 2014/07/08 13:39:47]

Done.

+        memset(payload(), 0, payloadSize());
+        setReuseMemory(); // Reuse memory for normal blink pages.

[Mads Ager (chromium), 2014/07/08 08:24:56]

Can we get rid of this reuse memory bit and just use the isLargeObject virtual
method?

[wibling-chromium, 2014/07/08 13:39:47]

Done.

+        BaseHeapPage::markOrphaned();
+    }
 
 protected:
     Header* findHeaderFromAddress(Address);
     void populateObjectStartBitMap();
     bool isObjectStartBitMapComputed() { return m_objectStartBitMapComputed; }
     TraceCallback traceCallback(Header*);
     bool hasVTable(Header*);
 
     HeapPage<Header>* m_next;
     ThreadHeap<Header>* m_heap;
@@ skipping 155 matching lines @@
     using GarbageCollectedFinalized<T>::operator delete;
 
 protected:
     ~ThreadSafeRefCountedGarbageCollected() { }
 
 private:
     OwnPtr<CrossThreadPersistent<T> > m_keepAlive;
     mutable Mutex m_mutex;
 };
 
+template<typename DataType>
+class HeapPool {
+protected:
+    HeapPool();
+
+    class PoolEntry {
+    public:
+        PoolEntry(DataType* data, PoolEntry* next)
+            : data(data)
+            , next(next)
+        { }
+
+        DataType* data;
+        PoolEntry* next;
+    };
+
+    PoolEntry* m_pool[NumberOfHeaps];
+};
+
+// Once pages have been used for one type of thread heap they will never be
+// reused for another type of thread heap. Instead of unmapping, we add the
+// pages to a pool of pages to be reused later by a thread heap of the same
+// type. This is done as a security feature to avoid type confusion. The
+// heaps are type segregated by having separate thread heaps for different
+// types of objects. Holding on to pages ensures that the same virtual address
+// space cannot be used for objects of another type than the type contained
+// in this page to begin with.
+class HeapMemoryPool : public HeapPool<PageMemory> {

[haraken, 2014/07/08 05:44:51]

It's a bit confusing to mix "HeapPool", "HeapMemoryPool", "OrphanedPagePool"
etc. We might want to rename:

HeapMemoryPool => PagePool
HeapPool => PagePoolBase

[wibling-chromium, 2014/07/08 13:39:46]

Good idea. I changed it to
 HeapPool => PagePool
 HeapMemoryPool = FreePagePool
 HeapOrphanedPagePool => OrphanedPagePool

+public:
+    ~HeapMemoryPool();
+    void addMemory(int index, PageMemory*);
+    PageMemory* takeMemory(int index);
+
+private:
+    Mutex m_mutex[NumberOfHeaps];
+};
+
+class HeapOrphanedPagePool : public HeapPool<BaseHeapPage> {
+public:
+    ~HeapOrphanedPagePool();
+    void addOrphanedPage(int, BaseHeapPage*);
+    void addOrphanedPages(int, Vector<BaseHeapPage*>&);
+    void decommitOrphanedPages();
+    bool contains(void*);
+};
+
 // The CallbackStack contains all the visitor callbacks used to trace and mark
 // objects. A specific CallbackStack instance contains at most bufferSize elements.
 // If more space is needed a new CallbackStack instance is created and chained
 // together with the former instance. I.e. a logical CallbackStack can be made of
 // multiple chained CallbackStack object instances.
 // There are two logical callback stacks. One containing all the marking callbacks and
 // one containing the weak pointer callbacks.
 class CallbackStack {
 public:
     CallbackStack(CallbackStack** first)
@@ skipping 30 matching lines @@
 
     static void init(CallbackStack** first);
     static void shutdown(CallbackStack** first);
     static void clear(CallbackStack** first)
     {
         if (!(*first)->isEmpty()) {
             shutdown(first);
             init(first);
         }
     }
-    bool popAndInvokeCallback(CallbackStack** first, Visitor*);
-    static void invokeCallbacks(CallbackStack** first, Visitor*);
+    template<bool ThreadLocal> bool popAndInvokeCallback(CallbackStack** first, Visitor*);
+    template<bool ThreadLocal> static void invokeCallbacks(CallbackStack** first, Visitor*);
 
     Item* allocateEntry(CallbackStack** first)
     {
         if (m_current < m_limit)
             return m_current++;
         return (new CallbackStack(first))->allocateEntry(first);
     }
 
 #ifndef NDEBUG
     bool hasCallbackForObject(const void*);
 #endif
 
 private:
-    void invokeOldestCallbacks(Visitor*);
+    template<bool ThreadLocal> void invokeOldestCallbacks(Visitor*);
 
     static const size_t bufferSize = 8000;
     Item m_buffer[bufferSize];
     Item* m_limit;
     Item* m_current;
     CallbackStack* m_next;
 };
 
 // Non-template super class used to pass a heap around to other classes.
 class BaseHeap {
@@ skipping 13 matching lines @@
     // and builds freelists for all the unused memory.
     virtual void sweep() = 0;
 
     // Forcefully finalize all objects in this part of the Blink heap
     // (potentially with the exception of one object). This is used
     // during thread termination to make sure that all objects for the
     // dying thread are finalized.
     virtual void assertEmpty() = 0;
 
     virtual void clearFreeLists() = 0;
-    virtual void clearMarks() = 0;
+    virtual void clearLiveAndMarkDead() = 0;
 #ifndef NDEBUG
     virtual void getScannedStats(HeapStats&) = 0;
 #endif
 
     virtual void makeConsistentForGC() = 0;
     virtual bool isConsistentForGC() = 0;
 
+    virtual void setShutdown() = 0;
+
     // Returns a bucket number for inserting a FreeListEntry of a
     // given size. All FreeListEntries in the given bucket, n, have
     // size >= 2^n.
     static int bucketIndexForSize(size_t);
 };
 
 // Thread heaps represent a part of the per-thread Blink heap.
 //
 // Each Blink thread has a number of thread heaps: one general heap
 // that contains any type of object and a number of heaps specialized
 // for specific object types (such as Node).
 //
 // Each thread heap contains the functionality to allocate new objects
 // (potentially adding new pages to the heap), to find and mark
 // objects during conservative stack scanning and to sweep the set of
 // pages after a GC.
 template<typename Header>
 class ThreadHeap : public BaseHeap {
 public:
-    ThreadHeap(ThreadState*);
+    ThreadHeap(ThreadState*, int);
     virtual ~ThreadHeap();
 
     virtual BaseHeapPage* heapPageFromAddress(Address);
 #if ENABLE(GC_TRACING)
     virtual const GCInfo* findGCInfoOfLargeHeapObject(Address);
 #endif
     virtual void sweep();
     virtual void assertEmpty();
     virtual void clearFreeLists();
-    virtual void clearMarks();
+    virtual void clearLiveAndMarkDead();
 #ifndef NDEBUG
     virtual void getScannedStats(HeapStats&);
 #endif
 
     virtual void makeConsistentForGC();
     virtual bool isConsistentForGC();
 
     ThreadState* threadState() { return m_threadState; }
     HeapStats& stats() { return m_threadState->stats(); }
     void flushHeapContainsCache()
     {
         m_threadState->heapContainsCache()->flush();
     }
 
     inline Address allocate(size_t, const GCInfo*);
     void addToFreeList(Address, size_t);
-    void addPageMemoryToPool(PageMemory*);
-    void addPageToPool(HeapPage<Header>*);
     inline static size_t roundedAllocationSize(size_t size)
     {
         return allocationSizeFromSize(size) - sizeof(Header);
     }
 
+    void setShutdown();
+    void removePageFromHeap(HeapPage<Header>*);
+
 private:
-    // Once pages have been used for one thread heap they will never
-    // be reused for another thread heap. Instead of unmapping, we add
-    // the pages to a pool of pages to be reused later by this thread
-    // heap. This is done as a security feature to avoid type
-    // confusion. The heap is type segregated by having separate
-    // thread heaps for various types of objects. Holding on to pages
-    // ensures that the same virtual address space cannot be used for
-    // objects of another type than the type contained in this thread
-    // heap.
-    class PagePoolEntry {
-    public:
-        PagePoolEntry(PageMemory* storage, PagePoolEntry* next)
-            : m_storage(storage)
-            , m_next(next)
-        { }
-
-        PageMemory* storage() { return m_storage; }
-        PagePoolEntry* next() { return m_next; }
-
-    private:
-        PageMemory* m_storage;
-        PagePoolEntry* m_next;
-    };
-
+    void addPageToHeap(const GCInfo*);
     PLATFORM_EXPORT Address outOfLineAllocate(size_t, const GCInfo*);
     static size_t allocationSizeFromSize(size_t);
-    void addPageToHeap(const GCInfo*);
     PLATFORM_EXPORT Address allocateLargeObject(size_t, const GCInfo*);
     Address currentAllocationPoint() const { return m_currentAllocationPoint; }
     size_t remainingAllocationSize() const { return m_remainingAllocationSize; }
     bool ownsNonEmptyAllocationArea() const { return currentAllocationPoint() && remainingAllocationSize(); }
     void setAllocationPoint(Address point, size_t size)
     {
         ASSERT(!point || heapPageFromAddress(point));
         ASSERT(size <= HeapPage<Header>::payloadSize());
         m_currentAllocationPoint = point;
         m_remainingAllocationSize = size;
     }
     void ensureCurrentAllocation(size_t, const GCInfo*);
     bool allocateFromFreeList(size_t);
 
     void freeLargeObject(LargeHeapObject<Header>*, LargeHeapObject<Header>**);
-
     void allocatePage(const GCInfo*);
-    PageMemory* takePageFromPool();
-    void clearPagePool();
-    void deletePages();
 
     Address m_currentAllocationPoint;
     size_t m_remainingAllocationSize;
 
     HeapPage<Header>* m_firstPage;
     LargeHeapObject<Header>* m_firstLargeHeapObject;
 
     int m_biggestFreeListIndex;
     ThreadState* m_threadState;
 
     // All FreeListEntries in the nth list have size >= 2^n.
     FreeListEntry* m_freeLists[blinkPageSizeLog2];
 
-    // List of pages that have been previously allocated, but are now
-    // unused.
-    PagePoolEntry* m_pagePool;
+    // Index into the memory pools. This is used so heaps of the same type (ie. index)
+    // put their pages into the correct pool for avoiding type confusion.

[haraken, 2014/07/08 05:44:51]

// This is used to ensure that the pages of the same type go into the correct
memory pool and thus avoid type confusion.

[wibling-chromium, 2014/07/08 13:39:46]

Done.

+    int m_index;
 };
 
 class PLATFORM_EXPORT Heap {
 public:
     static void init();
     static void shutdown();
     static void doShutdown();
 
     static BaseHeapPage* contains(Address);
     static BaseHeapPage* contains(void* pointer) { return contains(reinterpret_cast<Address>(pointer)); }
     static BaseHeapPage* contains(const void* pointer) { return contains(const_cast<void*>(pointer)); }
+#ifndef NDEBUG
+    static bool containedInHeapOrOrphanedPage(void*);
+#endif
 
     // Push a trace callback on the marking stack.
     static void pushTraceCallback(void* containerObject, TraceCallback);
 
     // Add a weak pointer callback to the weak callback work list. General
     // object pointer callbacks are added to a thread local weak callback work
     // list and the callback is called on the thread that owns the object, with
     // the closure pointer as an argument. Most of the time, the closure and
     // the containerObject can be the same thing, but the containerObject is
     // constrained to be on the heap, since the heap is used to identify the
     // correct thread.
     static void pushWeakObjectPointerCallback(void* closure, void* containerObject, WeakPointerCallback);
 
     // Similar to the more general pushWeakObjectPointerCallback, but cell
     // pointer callbacks are added to a static callback work list and the weak
     // callback is performed on the thread performing garbage collection. This
     // is OK because cells are just cleared and no deallocation can happen.
     static void pushWeakCellPointerCallback(void** cell, WeakPointerCallback);
 
     // Pop the top of the marking stack and call the callback with the visitor
     // and the object. Returns false when there is nothing more to do.
-    static bool popAndInvokeTraceCallback(Visitor*);
+    template<bool ThreadLocal> static bool popAndInvokeTraceCallback(Visitor*);

[Mads Ager (chromium), 2014/07/08 08:24:56]

Could you introduce an enum for this to make the call-sites easier to read?

enum GCType {
  NORMAL_GC,
  THREAD_LOCAL_GC
}

[wibling-chromium, 2014/07/08 13:39:46]

Done.

 
     // Remove an item from the weak callback work list and call the callback
     // with the visitor and the closure pointer. Returns false when there is
     // nothing more to do.
     static bool popAndInvokeWeakPointerCallback(Visitor*);
 
     // Register an ephemeron table for fixed-point iteration.
     static void registerWeakTable(void* containerObject, EphemeronCallback, EphemeronCallback);
 #ifndef NDEBUG
     static bool weakTableRegistered(const void*);
 #endif
 
     template<typename T> static Address allocate(size_t);
     template<typename T> static Address reallocate(void* previous, size_t);
 
     static void collectGarbage(ThreadState::StackState);
+    static void collectGarbageForThread(ThreadState*, bool);

[Mads Ager (chromium), 2014/07/08 08:24:56]

Maybe call this collectGarbageForTerminatingThread to make it a bit clearer that
this is not something that can be called to do a general GC for a thread. This
is only valid when the thread is dying and we therefore have the assumption that
there should be no real cross-thread pointers.

Could you add a name to the bool so it is clear for the signature what that bool
means?

(The rule about adding names to arguments is that you should not add a name if
it doesn't add anything, so don't add 'ThreadState* threadState' since that just
repeats that it is a thread state. However, bool doesn't mean anything unless it
has a name.)

[wibling-chromium, 2014/07/08 13:39:47]

Done.

     static void collectAllGarbage();
+    template<bool ThreadLocal> static void tracingAndGlobalWeakProcessing();

[Mads Ager (chromium), 2014/07/08 08:24:56]

traceAndPerformGlobalWeakProcessing?

[wibling-chromium, 2014/07/08 13:39:47]

Done. Changed it to traceRootsAndPerformGlobalWeakProcessing.

     static void setForcePreciseGCForTesting();
 
     static void prepareForGC();
 
     // Conservatively checks whether an address is a pointer in any of the thread
     // heaps. If so marks the object pointed to as live.
     static Address checkAndMarkPointer(Visitor*, Address);
 
 #if ENABLE(GC_TRACING)
     // Dump the path to specified object on the next GC. This method is to be invoked from GDB.
@@ skipping 17 matching lines @@
     static bool isConsistentForGC();
     static void makeConsistentForGC();
 
     static void flushHeapDoesNotContainCache();
     static bool heapDoesNotContainCacheIsEmpty() { return s_heapDoesNotContainCache->isEmpty(); }
 
     // Return true if the last GC found a pointer into a heap page
     // during conservative scanning.
     static bool lastGCWasConservative() { return s_lastGCWasConservative; }
 
+    static HeapMemoryPool* memoryPool() { return s_memoryPool; }
+    static HeapOrphanedPagePool* orphanedPagePool() { return s_orphanedPagePool; }
+
 private:
     static Visitor* s_markingVisitor;
 
     static CallbackStack* s_markingStack;
     static CallbackStack* s_weakCallbackStack;
     static CallbackStack* s_ephemeronStack;
     static HeapDoesNotContainCache* s_heapDoesNotContainCache;
     static bool s_shutdownCalled;
     static bool s_lastGCWasConservative;
+    static HeapMemoryPool* s_memoryPool;

[Mads Ager (chromium), 2014/07/08 08:24:56]

Do we want the page pool to be global or do we want that to be thread local?

[wibling-chromium, 2014/07/08 13:39:47]

There are trade-offs for both. With the global version we can reuse orphaned
pages whereas we cannot do that if we have a per thread free page pool. Reusing
ensures we are never using virtual adress space for an object of a different
type even when a thread dies.
The downside of the global version is the mutex, but I doubt that will be an
issue since it is only on page allocation which is relatively rare. If it
becomes an issue we can change it to thread local, but I would prefer to keep it
as is for this change.

+    static HeapOrphanedPagePool* s_orphanedPagePool;
     friend class ThreadState;
 };
 
 // The NoAllocationScope class is used in debug mode to catch unwanted
 // allocations. E.g. allocations during GC.
 template<ThreadAffinity Affinity>
 class NoAllocationScope {
 public:
     NoAllocationScope() : m_active(true) { enter(); }
 
@@ skipping 286 matching lines @@
 #define GC_PLUGIN_IGNORE(bug)                           \
     __attribute__((annotate("blink_gc_plugin_ignore")))
 #else
 #define STACK_ALLOCATED() DISALLOW_ALLOCATION()
 #define GC_PLUGIN_IGNORE(bug)
 #endif
 
 NO_SANITIZE_ADDRESS
 void HeapObjectHeader::checkHeader() const
 {
-    ASSERT(m_magic == magic);
+#ifndef NDEBUG
+    BaseHeapPage* page = pageHeaderFromObject(this);
+    ASSERT(page->orphaned() || m_magic == magic);
+#endif
 }
 
 Address HeapObjectHeader::payload()
 {
     return reinterpret_cast<Address>(this) + objectHeaderSize;
 }
 
 size_t HeapObjectHeader::payloadSize()
 {
     return size() - objectHeaderSize;
@@ skipping 998 matching lines @@
     // Use the payload size as recorded by the heap to determine how many
     // elements to finalize.
     size_t length = header->payloadSize() / sizeof(Value);
     Value* table = reinterpret_cast<Value*>(pointer);
     for (unsigned i = 0; i < length; i++) {
         if (!Table::isEmptyOrDeletedBucket(table[i]))
             table[i].~Value();
     }
 }
 
+template<bool ThreadLocal>
+bool CallbackStack::popAndInvokeCallback(CallbackStack** first, Visitor* visitor)

[Mads Ager (chromium), 2014/07/08 08:24:56]

We might want to keep this implementation in the cpp file. You should be able to
do that by forcing the two template instantiations in the cpp file.

template bool CallbackStack::popAndInvokeCallback<true>(args);
template bool CallbackStack::popAndInvokeCallback<false>(args);

We have such explicit instantiations in order to be able to have much of the
implementation in the cpp file already for the different object header types.

+{
+    if (m_current == &(m_buffer[0])) {
+        if (!m_next) {
+#ifndef NDEBUG
+            clearUnused();
+#endif
+            return false;
+        }
+        CallbackStack* nextStack = m_next;
+        *first = nextStack;
+        delete this;
+        return nextStack->popAndInvokeCallback<ThreadLocal>(first, visitor);
+    }
+    Item* item = --m_current;
+
+    // If the object being traced is located on a page which is dead don't
+    // trace it. This can happen when a conservative GC kept a dead object

[haraken, 2014/07/08 05:44:51]

// If the object being traced is located on a dead page, don't trace it. This
can happen when a conservative GC finds a pointer to the dead object.

+    // alive which pointed to a (now gone) object on the cleaned up page.
+    // Also if doing a thread local GC don't trace objects that are located

[haraken, 2014/07/08 05:44:51]

// Also if we're now performing a thread local GC and the object being traced is
located on another thread's heap, don't trace it.

+    // on other thread's heaps, ie. pages where the shuttingDown flag is not
+    // set.
+    BaseHeapPage* heapPage = pageHeaderFromObject(item->object());
+    if (ThreadLocal ? (heapPage->orphaned() || !heapPage->shuttingDown()) : heapPage->orphaned()) {

[haraken, 2014/07/08 05:44:51]

I'd rename "shuttingDown" to "isTargetOfThreadLocalGC" or something like that.

[haraken, 2014/07/08 05:44:51]

Would it be possible to add RELEASE_ASSERT(!heapPage->orphaned()) when we're
performing a precise GC? It will give us stack traces of the code we should fix.

+        // If tracing this from a global GC set the traced bit.
+        if (!ThreadLocal)
+            heapPage->setTraced();

[haraken, 2014/07/08 05:44:51]

Slightly more readable:

if (ThreadLocal) {
  if (heapPage->orphaned() || !heapPage->shuttingDown())
    return true;
} else {
  if (heapPage->orphaned()) {
    heapPage->setTraced();
    return true;
  }
}

+        return true;
+    }
+
+    VisitorCallback callback = item->callback();
+#if ENABLE(GC_TRACING)
+    if (ThreadState::isAnyThreadInGC()) // weak-processing will also use popAndInvokeCallback
+        visitor->setHostInfo(item->object(), classOf(item->object()));
+#endif
+    callback(visitor, item->object());
+
+    return true;
+}
+
 template<typename T, typename U, typename V, typename W, typename X>
 struct GCInfoTrait<HeapHashMap<T, U, V, W, X> > : public GCInfoTrait<HashMap<T, U, V, W, X, HeapAllocator> > { };
 template<typename T, typename U, typename V>
 struct GCInfoTrait<HeapHashSet<T, U, V> > : public GCInfoTrait<HashSet<T, U, V, HeapAllocator> > { };
 template<typename T, typename U, typename V>
 struct GCInfoTrait<HeapLinkedHashSet<T, U, V> > : public GCInfoTrait<LinkedHashSet<T, U, V, HeapAllocator> > { };
 template<typename T, size_t inlineCapacity, typename U>
 struct GCInfoTrait<HeapListHashSet<T, inlineCapacity, U> > : public GCInfoTrait<ListHashSet<T, inlineCapacity, U, HeapListHashSetAllocator<T, inlineCapacity> > > { };
 template<typename T, size_t inlineCapacity>
 struct GCInfoTrait<HeapVector<T, inlineCapacity> > : public GCInfoTrait<Vector<T, inlineCapacity, HeapAllocator> > { };
@@ skipping 12 matching lines @@
 };
 
 template<typename T>
 struct IfWeakMember<WeakMember<T> > {
     static bool isDead(Visitor* visitor, const WeakMember<T>& t) { return !visitor->isAlive(t.get()); }
 };
 
 }
 
 #endif // Heap_h