[oilpan]: Make thread shutdown more robust.

Source/platform/heap/HeapTest.cpp

 /*
  * 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 1679 matching lines @@
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     EXPECT_EQ(2, HeapTestSubClass::s_destructorCalls);
     EXPECT_EQ(3, HeapTestSuperClass::s_destructorCalls);
 }
 
 TEST(HeapTest, TypedHeapSanity)
 {
     // We use TraceCounter for allocating an object on the general heap.
     Persistent<TraceCounter> generalHeapObject = TraceCounter::create();
     Persistent<TestTypedHeapClass> typedHeapObject = TestTypedHeapClass::create();
-    EXPECT_NE(pageHeaderAddress(reinterpret_cast<Address>(generalHeapObject.get())),
-        pageHeaderAddress(reinterpret_cast<Address>(typedHeapObject.get())));
+    EXPECT_NE(pageHeaderFromObject(generalHeapObject.get()),
+        pageHeaderFromObject(typedHeapObject.get()));
 }
 
 TEST(HeapTest, NoAllocation)
 {
     EXPECT_TRUE(ThreadState::current()->isAllocationAllowed());
     {
         // Disallow allocation
         NoAllocationScope<AnyThread> noAllocationScope;
         EXPECT_FALSE(ThreadState::current()->isAllocationAllowed());
     }
@@ skipping 1710 matching lines @@
 {
     HeapStats initialHeapSize;
     IntWrapper::s_destructorCalls = 0;
 
     typedef HeapVector<Member<IntWrapper> > Vec;
     typedef PersistentHeapVector<Member<IntWrapper> > PVec;
     typedef PersistentHeapHashSet<Member<IntWrapper> > PSet;
     typedef PersistentHeapListHashSet<Member<IntWrapper> > PListSet;
     typedef PersistentHeapLinkedHashSet<Member<IntWrapper> > PLinkedSet;
     typedef PersistentHeapHashMap<Member<IntWrapper>, Member<IntWrapper> > PMap;
+    typedef PersistentHeapHashMap<WeakMember<IntWrapper>, Member<IntWrapper> > WeakPMap;
     typedef PersistentHeapDeque<Member<IntWrapper> > PDeque;
 
     clearOutOldGarbage(&initialHeapSize);
     {
         PVec pVec;
         PDeque pDeque;
         PSet pSet;
         PListSet pListSet;
         PLinkedSet pLinkedSet;
         PMap pMap;
+        WeakPMap wpMap;
 
         IntWrapper* one(IntWrapper::create(1));
         IntWrapper* two(IntWrapper::create(2));
         IntWrapper* three(IntWrapper::create(3));
         IntWrapper* four(IntWrapper::create(4));
         IntWrapper* five(IntWrapper::create(5));
         IntWrapper* six(IntWrapper::create(6));
         IntWrapper* seven(IntWrapper::create(7));
         IntWrapper* eight(IntWrapper::create(8));
         IntWrapper* nine(IntWrapper::create(9));
+        Persistent<IntWrapper> ten(IntWrapper::create(10));
+        IntWrapper* eleven(IntWrapper::create(11));
 
         pVec.append(one);
         pVec.append(two);
 
         pDeque.append(seven);
         pDeque.append(two);
 
         Vec* vec = new Vec();
         vec->swap(pVec);
 
         pVec.append(two);
         pVec.append(three);
 
         pSet.add(four);
         pListSet.add(eight);
         pLinkedSet.add(nine);
         pMap.add(five, six);
+        wpMap.add(ten, eleven);
 
         // Collect |vec| and |one|.
         vec = 0;
         Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
         EXPECT_EQ(1, IntWrapper::s_destructorCalls);
 
         EXPECT_EQ(2u, pVec.size());
         EXPECT_EQ(two, pVec.at(0));
         EXPECT_EQ(three, pVec.at(1));
 
         EXPECT_EQ(2u, pDeque.size());
         EXPECT_EQ(seven, pDeque.first());
         EXPECT_EQ(seven, pDeque.takeFirst());
         EXPECT_EQ(two, pDeque.first());
 
         EXPECT_EQ(1u, pDeque.size());
 
         EXPECT_EQ(1u, pSet.size());
         EXPECT_TRUE(pSet.contains(four));
 
         EXPECT_EQ(1u, pListSet.size());
         EXPECT_TRUE(pListSet.contains(eight));
 
         EXPECT_EQ(1u, pLinkedSet.size());
         EXPECT_TRUE(pLinkedSet.contains(nine));
 
         EXPECT_EQ(1u, pMap.size());
         EXPECT_EQ(six, pMap.get(five));
+
+        EXPECT_EQ(1u, wpMap.size());
+        EXPECT_EQ(eleven, wpMap.get(ten));
+        ten.clear();
+        Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
+        EXPECT_EQ(0u, wpMap.size());
     }
 
     // Collect previous roots.
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
-    EXPECT_EQ(9, IntWrapper::s_destructorCalls);
+    EXPECT_EQ(11, IntWrapper::s_destructorCalls);
 }
 
 TEST(HeapTest, CollectionNesting)
 {
     HeapStats initialStats;
     clearOutOldGarbage(&initialStats);
     int* key = &IntWrapper::s_destructorCalls;
     IntWrapper::s_destructorCalls = 0;
     typedef HeapVector<Member<IntWrapper> > IntVector;
     typedef HeapDeque<Member<IntWrapper> > IntDeque;
@@ skipping 1064 matching lines @@
     EXPECT_EQ(lifeObject, map->get(lifeObject)->link());
     deadObject.clear(); // Now it can live up to its name.
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     EXPECT_EQ(1u, map->size());
     EXPECT_EQ(lifeObject, map->get(lifeObject)->link());
     lifeObject.clear(); // Despite its name.
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     EXPECT_EQ(0u, map->size());
 }
 
+class CrossThreadObject : public GarbageCollectedFinalized<CrossThreadObject> {
+public:
+    static CrossThreadObject* create(IntWrapper* workerObjectPointer)
+    {
+        return new CrossThreadObject(workerObjectPointer);
+    }
+
+    virtual ~CrossThreadObject()
+    {
+        ++s_destructorCalls;
+    }
+
+    static int s_destructorCalls;
+    void trace(Visitor* visitor) { visitor->trace(m_workerObject); }
+
+private:
+    CrossThreadObject(IntWrapper* workerObjectPointer) : m_workerObject(workerObjectPointer) { }
+
+private:
+    Member<IntWrapper> m_workerObject;
+};
+
+int CrossThreadObject::s_destructorCalls = 0;
+
+class CrossThreadPointerTester {
+public:
+    static void test()
+    {
+        CrossThreadObject::s_destructorCalls = 0;
+        IntWrapper::s_destructorCalls = 0;
+        createThread(&workerThreadMain, 0, "Worker Thread");
+
+        // Wait for the worker to run.
+        while (!s_workerRunning) {
+            yield();
+        }
+
+        uintptr_t stackPtrValue = 0;
+        {
+            // Create an object with a pointer to the other heap's IntWrapper.
+            Persistent<CrossThreadObject> cto = CrossThreadObject::create(const_cast<IntWrapper*>(s_workerObjectPointer));
+            s_workerObjectPointer = 0;
+
+            Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
+
+            // Nothing should have been collected/destructed.
+            EXPECT_EQ(0, CrossThreadObject::s_destructorCalls);
+            EXPECT_EQ(0, IntWrapper::s_destructorCalls);
+
+            // Put cto into a stack value. This is used to check that a conservative
+            // GC succeeds even though we are tracing the other thread heap after
+            // shutting it down.
+            stackPtrValue = reinterpret_cast<uintptr_t>(cto.get());
+        }
+
+        // At this point it is "programatically" okay to shut down the worker thread
+        // since the cto object should be dead. However out stackPtrValue will cause a
+        // trace of the object when doing a conservative GC.
+        // The worker thread's thread local GC's should just add the worker thread's
+        // pages to the heap after finalizing IntWrapper.
+        s_workerFinish = true;
+
+        // Wait for the worker to finish.
+        while (s_workerRunning) {
+            // No need to enter a safepoint since the GC's done by the worker at detach
+            // are thread local and doesn't require other threads to enter a safepoint.
+            yield();
+        }
+        // After the worker thread has detached it should have finalized the
+        // IntWrapper object on its heaps. Since there has been no global GC
+        // the cto object should not have been finalized.
+        EXPECT_EQ(0, CrossThreadObject::s_destructorCalls);
+        EXPECT_EQ(1, IntWrapper::s_destructorCalls);
+
+        // Now do a conservative GC. The stackPtrValue should keep cto alive
+        // and will also cause the orphaned page of the other thread to be
+        // traced. At this point cto should still not be finalized.
+        Heap::collectGarbage(ThreadState::HeapPointersOnStack);
+        EXPECT_EQ(0, CrossThreadObject::s_destructorCalls);
+        EXPECT_EQ(1, IntWrapper::s_destructorCalls);
+
+        // Set stackPtrValue to zero and see the cto object being collected in the
+        // conservative GC.
+        stackPtrValue = 0;
+        Heap::collectGarbage(ThreadState::HeapPointersOnStack);
+        EXPECT_EQ(1, CrossThreadObject::s_destructorCalls);
+        EXPECT_EQ(1, IntWrapper::s_destructorCalls);
+    }
+
+private:
+    static void workerThreadMain(void* data)
+    {
+        ThreadState::attach();
+
+        {
+            // Create a worker object that is only kept alive by a cross thread
+            // pointer (from CrossThreadObject).
+            IntWrapper* workerObject = IntWrapper::create(42);
+            s_workerObjectPointer = workerObject;
+            s_workerRunning = true;
+        }
+
+        // Wait for main thread to signal the worker to finish.
+        while (!s_workerFinish) {
+            ThreadState::current()->safePoint(ThreadState::NoHeapPointersOnStack);
+            yield();
+        }
+
+        ThreadState::detach();
+        s_workerRunning = false;
+    }
+
+    static volatile IntWrapper* s_workerObjectPointer;
+    static volatile bool s_workerRunning;
+    static volatile bool s_workerFinish;
+};
+
+volatile bool CrossThreadPointerTester::s_workerRunning = false;
+volatile bool CrossThreadPointerTester::s_workerFinish = false;
+volatile IntWrapper* CrossThreadPointerTester::s_workerObjectPointer = 0;
+
+TEST(HeapTest, CrossThreadPointerToOrphanedPage)
+{
+    CrossThreadPointerTester::test();
+}
+
+class DeadBitTester {
+public:
+    static void test()
+    {
+        IntWrapper::s_destructorCalls = 0;
+        createThread(&workerThreadMain, 0, "Worker Thread");
+
+        // Wait for the worker to run.
+        while (!s_workerRunning) {
+            yield();
+        }
+
+        // Do a GC. This will not find the worker threads object since it
+        // is not referred from any of the threads. Even a conservative
+        // GC will not find it.
+        Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
+
+        // Since the worker thread is not sweeping the worker object should
+        // not have been finalized.
+        EXPECT_EQ(0, IntWrapper::s_destructorCalls);
+
+        // Put the worker thread's object address on the stack and do a
+        // conservative GC. This should find the worker object, but since
+        // it was dead in the previous GC it should not be traced in this
+        // GC.
+        uintptr_t stackPtrValue = s_workerObjectPointer;
+        s_workerObjectPointer = 0;
+        ASSERT_UNUSED(stackPtrValue, stackPtrValue);
+        Heap::collectGarbage(ThreadState::HeapPointersOnStack);
+
+        // Since the worker thread is not sweeping the worker object should
+        // not have been finalized.
+        EXPECT_EQ(0, IntWrapper::s_destructorCalls);
+
+        // Tell the worker thread to finish its busy loop, but not detach.
+        // This means there is no further GC, however the worker thread gets
+        // to sweep so the worker object should now be finalized since the
+        // above conservative GC should not have traced the object that was
+        // found dead in the first GC.
+        s_workerContinue = true;
+
+        // Wait for the worker to sweep.
+        while (!s_workerDidSweep) {
+            // No need to enter a safepoint since the worker thread is just sweeping
+            // its own heap.
+            yield();
+        }
+        EXPECT_EQ(1, IntWrapper::s_destructorCalls);
+
+        // Tell the worker to shut down.
+        s_workerFinish = true;
+    }
+
+private:
+    static void workerThreadMain(void* data)
+    {
+        ThreadState::attach();
+
+        {
+            // Create a worker object that is not kept alive except the
+            // main thread will keep it as an integer value on its stack.
+            IntWrapper* workerObject = IntWrapper::create(42);
+            s_workerObjectPointer = reinterpret_cast<uintptr_t>(workerObject);
+            s_workerRunning = true;
+        }
+
+        // Enter a safepoint, but don't exit until the main thread tells us it
+        // has done it works. This means this thread will not sweep its heap
+        // since the sweep is done when the safepoint scope is destructed.
+        {
+            ThreadState::SafePointScope scope(ThreadState::NoHeapPointersOnStack);
+            while (!s_workerContinue) {
+                yield();
+            }
+        }
+
+        // At this point the worker have swept its heaps. Tell the main thread its
+        // done.
+        s_workerDidSweep = true;
+
+        // Wait with detach until the main thread says so. This is not strictly
+        // necessary, but it means the worker thread will not do its thread local
+        // GCs just yet, making it easier to reason about that no new GC has occurred
+        // and the above sweep was the one finalizing the worker object.
+        while (!s_workerFinish) {
+            yield();
+        }
+
+        ThreadState::detach();
+        s_workerRunning = false;
+    }
+
+    static volatile bool s_workerRunning;

[Mads Ager (chromium), 2014/07/11 10:59:34]

I guess we should either use atomic ops here or have mutex protection when
updating? Since the tests are not performance critical I would just add mutex
protection for the access to these variables?

[wibling-chromium, 2014/07/11 13:06:54]

Done.

+    static volatile bool s_workerContinue;
+    static volatile bool s_workerDidSweep;
+    static volatile bool s_workerFinish;
+    static volatile uintptr_t s_workerObjectPointer;
+};
+
+volatile bool DeadBitTester::s_workerRunning = false;
+volatile bool DeadBitTester::s_workerContinue = false;
+volatile bool DeadBitTester::s_workerDidSweep = false;
+volatile bool DeadBitTester::s_workerFinish = false;
+volatile uintptr_t DeadBitTester::s_workerObjectPointer = 0;
+
+TEST(HeapTest, ObjectDeadBit)
+{
+    DeadBitTester::test();
+}
 
 } // WebCore namespace