Changeset 73545 in webkit for trunk/JavaScriptCore/runtime

Timestamp:

Dec 8, 2010, 1:44:38 PM (14 years ago)

Author:

[email protected]

Message:

2010-12-08 Oliver Hunt <​[email protected]>

Reviewed by Gavin Barraclough.

Marking the active global object re-enters through markConservatively
​https://bugs.webkit.org/show_bug.cgi?id=50711

draining of the MarkStack is not allowed to be re-entrant, we got away
with this simply due to the logic in MarkStack::drain implicitly handling
changes that could be triggered by the re-entry.

Just to be safe this patch removes the re-entry through markConservatively
so we don't accidentally introduce such an issue in future. I've also
added an assertion to catch such errors.

• runtime/Collector.cpp: (JSC::Heap::markConservatively): (JSC::Heap::markCurrentThreadConservativelyInternal): (JSC::Heap::markOtherThreadConservatively):
• runtime/JSArray.h: (JSC::MarkStack::drain):
• runtime/MarkStack.h: (JSC::MarkStack::MarkStack):

Location:

trunk/JavaScriptCore/runtime

Files:

• Collector.cpp (modified) (3 diffs)
• JSArray.h (modified) (2 diffs)
• MarkStack.h (modified) (2 diffs)

trunk/JavaScriptCore/runtime/Collector.cpp

@@ -686 +686 @@
                     continue;
                 markStack.append(reinterpret_cast<JSCell*>(xAsBits));
-                markStack.drain();
             }
         }
@@ -698 +697 @@
     void* stackBase = currentThreadStackBase();
     markConservatively(markStack, stackPointer, stackBase);
+    markStack.drain();
 }
 
@@ -860 +860 @@
     // mark the thread's registers
     markConservatively(markStack, static_cast<void*>(&regs), static_cast<void*>(reinterpret_cast<char*>(&regs) + regSize));
+    markStack.drain();
 
     void* stackPointer = otherThreadStackPointer(regs);
     markConservatively(markStack, stackPointer, thread->stackBase);
+    markStack.drain();
 
     resumeThread(thread->platformThread);

trunk/JavaScriptCore/runtime/JSArray.h

@@ -223 +223 @@
     inline void MarkStack::drain()
     {
+#if !ASSERT_DISABLED
+        ASSERT(!m_isDraining);
+        m_isDraining = true;
+#endif
         while (!m_markSets.isEmpty() || !m_values.isEmpty()) {
             while (!m_markSets.isEmpty() && m_values.size() < 50) {
@@ -261 +265 @@
                 markChildren(m_values.removeLast());
         }
+#if !ASSERT_DISABLED
+        m_isDraining = false;
+#endif
     }
 

trunk/JavaScriptCore/runtime/MarkStack.h

@@ -42 +42 @@
         MarkStack(void* jsArrayVPtr)
             : m_jsArrayVPtr(jsArrayVPtr)
-#ifndef NDEBUG
+#if !ASSERT_DISABLED
             , m_isCheckingForDefaultMarkViolation(false)
+            , m_isDraining(false)
 #endif
         {
@@ -179 +180 @@
         static size_t s_pageSize;
 
-#ifndef NDEBUG
+#if !ASSERT_DISABLED
     public:
         bool m_isCheckingForDefaultMarkViolation;
+        bool m_isDraining;
 #endif
     };