Update thread-local weak processing comments following r397904.

third_party/WebKit/Source/platform/heap/ThreadState.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 403 matching lines @@
 
 bool ThreadState::popAndInvokeThreadLocalWeakCallback(Visitor* visitor)
 {
     ASSERT(checkThread());
     // For weak processing we should never reach orphaned pages since orphaned
     // pages are not traced and thus objects on those pages are never be
     // registered as objects on orphaned pages. We cannot assert this here since
     // we might have an off-heap collection. We assert it in
     // ThreadHeap::pushThreadLocalWeakCallback.
     if (CallbackStack::Item* item = m_threadLocalWeakCallbackStack->pop()) {
-        // Note that the thread-local weak processing can be called for
-        // an already dead object (for which isHeapObjectAlive(object) can
-        // return false). This can happen in the following scenario:
-        //
-        // 1) Marking runs. A weak callback for an object X is registered
-        //    to the thread that created the object X (say, thread P).
-        // 2) Marking finishes. All other threads are resumed.
-        // 3) The object X becomes unreachable.
-        // 4) A next GC hits before the thread P wakes up.
-        // 5) Marking runs. The object X is not marked.
-        // 6) Marking finishes. All other threads are resumed.
-        // 7) The thread P wakes up and invokes pending weak callbacks.
-        //    The weak callback for the object X is called, but the object X
-        //    is already dead.
-        //
-        // Even in this case, it is safe to access the object X in the weak
-        // callback because it is not yet swept. It is completely wasteful
-        // to invoke the weak callback for dead objects but it is just
-        // wasteful and safe.
-        //
-        // TODO(Oilpan): Avoid calling weak callbacks for dead objects.
-        // We can do that by checking isHeapObjectAlive(object) before
-        // calling the weak callback, but in that case Callback::Item
-        // needs to understand T*.
         item->call(visitor);
         return true;
     }
     return false;
 }
 
 void ThreadState::threadLocalWeakProcessing()
 {
     ASSERT(checkThread());
     ASSERT(!sweepForbidden());
@@ skipping 499 matching lines @@
         m_arenas[i]->makeConsistentForMutator();
 }
 
 void ThreadState::preGC()
 {
     ASSERT(!isInGC());
     setGCState(GCRunning);
     makeConsistentForGC();
     flushHeapDoesNotContainCacheIfNeeded();
     clearArenaAges();
+
+    // It is possible, albeit rare, for a thread to be kept
+    // at a safepoint across multiple GCs, as resuming all attached
+    // threads after the "global" GC phases will contend for the shared
+    // safepoint barrier mutexes etc, which can additionally delay
+    // a thread. Enough so that another thread may initiate
+    // a new GC before this has happened.
+    //
+    // In which case the parked thread's ThreadState will have unprocessed
+    // entries on its local weak callback stack when that later GC goes
+    // ahead. Clear out and invalidate the stack now, as the thread
+    // should only process callbacks that's found to be reachable by
+    // the latest GC, when it eventually gets to next perform
+    // thread-local weak processing.
     m_threadLocalWeakCallbackStack->clear();
 }
 
 void ThreadState::postGC(BlinkGC::GCType gcType)
 {
     ASSERT(isInGC());
     for (int i = 0; i < BlinkGC::NumberOfArenas; i++)
         m_arenas[i]->prepareForSweep();
 
     if (gcType == BlinkGC::GCWithSweep) {
@@ skipping 552 matching lines @@
     threadDump->AddScalar("dead_count", "objects", totalDeadCount);
     threadDump->AddScalar("live_size", "bytes", totalLiveSize);
     threadDump->AddScalar("dead_size", "bytes", totalDeadSize);
 
     base::trace_event::MemoryAllocatorDump* heapsDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(heapsDumpName);
     base::trace_event::MemoryAllocatorDump* classesDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(classesDumpName);
     BlinkGCMemoryDumpProvider::instance()->currentProcessMemoryDump()->AddOwnershipEdge(classesDump->guid(), heapsDump->guid());
 }
 
 } // namespace blink