Title: [148475] trunk/Source/_javascript_Core
Revision
148475
Author
mhahnenb...@apple.com
Date
2013-04-15 16:17:51 -0700 (Mon, 15 Apr 2013)

Log Message

HeapTimer lifetime should be less complicated
https://bugs.webkit.org/show_bug.cgi?id=114529

Reviewed by Oliver Hunt.

Right now our HeapTimer lifetime is rather complicated. HeapTimers are "owned" by the JSGlobalData,
but there's an issue in that there can be races between a thread that is trying to tear down a JSGlobalData
and the HeapTimer's fire function. Our current code for tearing down HeapTimers is an intricate and delicate
dance which probably contains subtle bugs.

We can make our lives easier by changing things around a bit.

1) We should free the API lock from being solely owned by the JSGlobalData so we don't have to worry about
   grabbing the lock out of invalid memory when our HeapTimer callback fires.

2) We should also make it so that we deref the JSGlobalData first, then unlock the API lock so that when we
   have the lock, the JSGlobalData is in one of two states: fully valid or completely destroyed, and we know exactly which one.

3) The JSLock can tell us this information by keeping a back pointer to the JSGlobalData. When the JSGlobalData's
   destructor is called, it clears this pointer in the JSLock. Other clients of the API lock can then check
   this pointer to determine whether or not the JSGlobalData is still around.

4) The CFRunLoopTimer will use the API lock as its context rather than the HeapTimer itself. The only way
   the HeapTimer's callback can get to the HeapTimer is through the API lock's JSGlobalData pointer.

5) The CFRunLoopTimerContext struct has two fields for retain and release callbacks for the context's info field.
   We'll provide these callbacks to ref() and deref() the JSLock as necessary. Thus, the timer becomes the other
   owner of the JSLock apart from the JSGlobalData.

* API/APIShims.h: Remove the cruft that was required by the previous design, such as RefGlobalDataTag.
(JSC::APIEntryShimWithoutLock::APIEntryShimWithoutLock):
(JSC::APIEntryShimWithoutLock::~APIEntryShimWithoutLock):
(APIEntryShimWithoutLock):
(JSC::APIEntryShim::APIEntryShim):
(JSC::APIEntryShim::~APIEntryShim): Protect the API lock with a RefPtr, deref the JSGlobalData, which could destroy it,
then unlock the API lock. This ordering prevents others from obtaining the API lock while the JSGlobalData is in the
middle of being torn down.
(JSC::APIEntryShim::init): We now take the lock, then ref the JSGlobalData, which is the opposite order of when we
tear down the shim.
* heap/Heap.cpp:
(JSC::Heap::setActivityCallback): Use PassOwnPtr now.
(JSC::Heap::activityCallback): Ditto.
(JSC::Heap::sweeper): Ditto.
(JSC):
* heap/Heap.h:
(Heap):
* heap/HeapTimer.cpp:
(JSC::retainAPILock): Retain callback for CFRunLoopTimerContext struct.
(JSC::releaseAPILock): Release callback for the CFRunLoopTimerContext struct.
(JSC::HeapTimer::HeapTimer): Use the API lock as the context's info field rather than the HeapTimer.
(JSC::HeapTimer::timerDidFire): Grab the API lock. Return early if the JSGlobalData has already been destroyed.
Otherwise, figure out which kind of HeapTimer we are based on the CFRunLoopTimerRef passed to the callback and
call the HeapTimer's callback.
* heap/HeapTimer.h:
(HeapTimer):
* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::create): PassOwnPtr all the things.
* heap/IncrementalSweeper.h:
(IncrementalSweeper):
* jsc.cpp:
(jscmain): We use an APIEntryShim instead of a RefPtr for the JSGlobalData because we need to
tear down the JSGlobalData while we still hold the lock, which the APIEntryShim handles correctly.
* runtime/GCActivityCallback.h:
(DefaultGCActivityCallback):
(JSC::DefaultGCActivityCallback::create):
* runtime/JSGlobalData.cpp:
(JSC::JSGlobalData::JSGlobalData):
(JSC::JSGlobalData::~JSGlobalData): Notify the API lock that the JSGlobalData is being torn down.
* runtime/JSGlobalData.h:
(JSGlobalData):
(JSC::JSGlobalData::apiLock):
* runtime/JSLock.cpp:
(JSC::JSLockHolder::JSLockHolder): Ref, then lock (just like the API shim).
(JSC):
(JSC::JSLock::willDestroyGlobalData):
(JSC::JSLockHolder::init):
(JSC::JSLockHolder::~JSLockHolder): Protect, deref, then unlock (just like the API shim).
(JSC::JSLock::JSLock):
* runtime/JSLock.h: Add back pointer to the JSGlobalData and a callback for when the JSGlobalData is being
torn down that clears this pointer to notify other clients (i.e. timer callbacks) that the JSGlobalData is no
longer valid.
(JSLockHolder):
(JSLock):
(JSC::JSLock::globalData):
* testRegExp.cpp:
(realMain): We use an APIEntryShim instead of a RefPtr for the JSGlobalData because we need to
tear down the JSGlobalData while we still hold the lock, which the APIEntryShim handles correctly.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/API/APIShims.h (148474 => 148475)


--- trunk/Source/_javascript_Core/API/APIShims.h	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/API/APIShims.h	2013-04-15 23:17:51 UTC (rev 148475)
@@ -35,18 +35,11 @@
 namespace JSC {
 
 class APIEntryShimWithoutLock {
-public:
-    enum RefGlobalDataTag { DontRefGlobalData = 0, RefGlobalData };
-
 protected:
-    APIEntryShimWithoutLock(JSGlobalData* globalData, bool registerThread, RefGlobalDataTag shouldRefGlobalData)
-        : m_shouldRefGlobalData(shouldRefGlobalData)
-        , m_globalData(globalData)
+    APIEntryShimWithoutLock(JSGlobalData* globalData, bool registerThread)
+        : m_globalData(globalData)
         , m_entryIdentifierTable(wtfThreadData().setCurrentIdentifierTable(globalData->identifierTable))
     {
-        if (shouldRefGlobalData)
-            m_globalData->ref();
-        UNUSED_PARAM(registerThread);
         if (registerThread)
             globalData->heap.machineThreads().addCurrentThread();
         m_globalData->heap.activityCallback()->synchronize();
@@ -56,13 +49,10 @@
     ~APIEntryShimWithoutLock()
     {
         wtfThreadData().setCurrentIdentifierTable(m_entryIdentifierTable);
-        if (m_shouldRefGlobalData)
-            m_globalData->deref();
     }
 
 protected:
-    RefGlobalDataTag m_shouldRefGlobalData;
-    JSGlobalData* m_globalData;
+    RefPtr<JSGlobalData> m_globalData;
     IdentifierTable* m_entryIdentifierTable;
 };
 
@@ -70,36 +60,26 @@
 public:
     // Normal API entry
     APIEntryShim(ExecState* exec, bool registerThread = true)
-        : APIEntryShimWithoutLock(&exec->globalData(), registerThread, RefGlobalData)
+        : APIEntryShimWithoutLock(&exec->globalData(), registerThread)
+        , m_lockHolder(exec)
     {
-        init();
     }
 
-    // This constructor is necessary for HeapTimer to prevent it from accidentally resurrecting 
-    // the ref count of a "dead" JSGlobalData.
-    APIEntryShim(JSGlobalData* globalData, RefGlobalDataTag refGlobalData, bool registerThread = true)
-        : APIEntryShimWithoutLock(globalData, registerThread, refGlobalData)
-    {
-        init();
-    }
-
     // JSPropertyNameAccumulator only has a globalData.
     APIEntryShim(JSGlobalData* globalData, bool registerThread = true)
-        : APIEntryShimWithoutLock(globalData, registerThread, RefGlobalData)
+        : APIEntryShimWithoutLock(globalData, registerThread)
+        , m_lockHolder(globalData)
     {
-        init();
     }
 
     ~APIEntryShim()
     {
-        m_globalData->apiLock().unlock();
+        // Destroying our JSLockHolder should also destroy the JSGlobalData.
+        m_globalData.clear();
     }
 
 private:
-    void init()
-    {
-        m_globalData->apiLock().lock();
-    }
+    JSLockHolder m_lockHolder;
 };
 
 class APICallbackShim {

Modified: trunk/Source/_javascript_Core/ChangeLog (148474 => 148475)


--- trunk/Source/_javascript_Core/ChangeLog	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-04-15 23:17:51 UTC (rev 148475)
@@ -1,3 +1,93 @@
+2013-04-15  Mark Hahnenberg  <mhahnenb...@apple.com>
+
+        HeapTimer lifetime should be less complicated
+        https://bugs.webkit.org/show_bug.cgi?id=114529
+
+        Reviewed by Oliver Hunt.
+
+        Right now our HeapTimer lifetime is rather complicated. HeapTimers are "owned" by the JSGlobalData, 
+        but there's an issue in that there can be races between a thread that is trying to tear down a JSGlobalData 
+        and the HeapTimer's fire function. Our current code for tearing down HeapTimers is an intricate and delicate 
+        dance which probably contains subtle bugs.
+
+        We can make our lives easier by changing things around a bit. 
+
+        1) We should free the API lock from being solely owned by the JSGlobalData so we don't have to worry about 
+           grabbing the lock out of invalid memory when our HeapTimer callback fires. 
+
+        2) We should also make it so that we deref the JSGlobalData first, then unlock the API lock so that when we 
+           have the lock, the JSGlobalData is in one of two states: fully valid or completely destroyed, and we know exactly which one. 
+
+        3) The JSLock can tell us this information by keeping a back pointer to the JSGlobalData. When the JSGlobalData's 
+           destructor is called, it clears this pointer in the JSLock. Other clients of the API lock can then check 
+           this pointer to determine whether or not the JSGlobalData is still around.
+
+        4) The CFRunLoopTimer will use the API lock as its context rather than the HeapTimer itself. The only way 
+           the HeapTimer's callback can get to the HeapTimer is through the API lock's JSGlobalData pointer.
+
+        5) The CFRunLoopTimerContext struct has two fields for retain and release callbacks for the context's info field. 
+           We'll provide these callbacks to ref() and deref() the JSLock as necessary. Thus, the timer becomes the other 
+           owner of the JSLock apart from the JSGlobalData.
+
+        * API/APIShims.h: Remove the cruft that was required by the previous design, such as RefGlobalDataTag.
+        (JSC::APIEntryShimWithoutLock::APIEntryShimWithoutLock):
+        (JSC::APIEntryShimWithoutLock::~APIEntryShimWithoutLock):
+        (APIEntryShimWithoutLock):
+        (JSC::APIEntryShim::APIEntryShim):
+        (JSC::APIEntryShim::~APIEntryShim): Protect the API lock with a RefPtr, deref the JSGlobalData, which could destroy it,
+        then unlock the API lock. This ordering prevents others from obtaining the API lock while the JSGlobalData is in the 
+        middle of being torn down.
+        (JSC::APIEntryShim::init): We now take the lock, then ref the JSGlobalData, which is the opposite order of when we 
+        tear down the shim.
+        * heap/Heap.cpp:
+        (JSC::Heap::setActivityCallback): Use PassOwnPtr now.
+        (JSC::Heap::activityCallback): Ditto.
+        (JSC::Heap::sweeper): Ditto.
+        (JSC):
+        * heap/Heap.h:
+        (Heap):
+        * heap/HeapTimer.cpp:
+        (JSC::retainAPILock): Retain callback for CFRunLoopTimerContext struct.
+        (JSC::releaseAPILock): Release callback for the CFRunLoopTimerContext struct.
+        (JSC::HeapTimer::HeapTimer): Use the API lock as the context's info field rather than the HeapTimer.
+        (JSC::HeapTimer::timerDidFire): Grab the API lock. Return early if the JSGlobalData has already been destroyed.
+        Otherwise, figure out which kind of HeapTimer we are based on the CFRunLoopTimerRef passed to the callback and 
+        call the HeapTimer's callback.
+        * heap/HeapTimer.h:
+        (HeapTimer):
+        * heap/IncrementalSweeper.cpp:
+        (JSC::IncrementalSweeper::create): PassOwnPtr all the things.
+        * heap/IncrementalSweeper.h:
+        (IncrementalSweeper):
+        * jsc.cpp:
+        (jscmain): We use an APIEntryShim instead of a RefPtr for the JSGlobalData because we need to 
+        tear down the JSGlobalData while we still hold the lock, which the APIEntryShim handles correctly.
+        * runtime/GCActivityCallback.h:
+        (DefaultGCActivityCallback):
+        (JSC::DefaultGCActivityCallback::create):
+        * runtime/JSGlobalData.cpp:
+        (JSC::JSGlobalData::JSGlobalData):
+        (JSC::JSGlobalData::~JSGlobalData): Notify the API lock that the JSGlobalData is being torn down.
+        * runtime/JSGlobalData.h:
+        (JSGlobalData):
+        (JSC::JSGlobalData::apiLock):
+        * runtime/JSLock.cpp:
+        (JSC::JSLockHolder::JSLockHolder): Ref, then lock (just like the API shim).
+        (JSC):
+        (JSC::JSLock::willDestroyGlobalData):
+        (JSC::JSLockHolder::init):
+        (JSC::JSLockHolder::~JSLockHolder): Protect, deref, then unlock (just like the API shim).
+        (JSC::JSLock::JSLock):
+        * runtime/JSLock.h: Add back pointer to the JSGlobalData and a callback for when the JSGlobalData is being
+        torn down that clears this pointer to notify other clients (i.e. timer callbacks) that the JSGlobalData is no
+        longer valid.
+        (JSLockHolder):
+        (JSLock):
+        (JSC::JSLock::globalData):
+        * testRegExp.cpp:
+        (realMain): We use an APIEntryShim instead of a RefPtr for the JSGlobalData because we need to 
+        tear down the JSGlobalData while we still hold the lock, which the APIEntryShim handles correctly.
+
 2013-04-15  Julien Brianceau  <jbrianc...@nds.com>
 
         LLInt SH4 backend implementation

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (148474 => 148475)


--- trunk/Source/_javascript_Core/heap/Heap.cpp	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2013-04-15 23:17:51 UTC (rev 148475)
@@ -809,19 +809,19 @@
     m_objectSpace.forEachDeadCell<MarkObject>();
 }
 
-void Heap::setActivityCallback(GCActivityCallback* activityCallback)
+void Heap::setActivityCallback(PassOwnPtr<GCActivityCallback> activityCallback)
 {
     m_activityCallback = activityCallback;
 }
 
 GCActivityCallback* Heap::activityCallback()
 {
-    return m_activityCallback;
+    return m_activityCallback.get();
 }
 
 IncrementalSweeper* Heap::sweeper()
 {
-    return m_sweeper;
+    return m_sweeper.get();
 }
 
 void Heap::setGarbageCollectionTimerEnabled(bool enable)
@@ -864,15 +864,6 @@
     m_compiledCode.append(executable);
 }
 
-void Heap::didStartVMShutdown()
-{
-    m_activityCallback->didStartVMShutdown();
-    m_activityCallback = 0;
-    m_sweeper->didStartVMShutdown();
-    m_sweeper = 0;
-    lastChanceToFinalize();
-}
-
 class Zombify : public MarkedBlock::VoidFunctor {
 public:
     void operator()(JSCell* cell)

Modified: trunk/Source/_javascript_Core/heap/Heap.h (148474 => 148475)


--- trunk/Source/_javascript_Core/heap/Heap.h	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2013-04-15 23:17:51 UTC (rev 148475)
@@ -105,7 +105,7 @@
         MachineThreads& machineThreads() { return m_machineThreads; }
 
         JS_EXPORT_PRIVATE GCActivityCallback* activityCallback();
-        JS_EXPORT_PRIVATE void setActivityCallback(GCActivityCallback*);
+        JS_EXPORT_PRIVATE void setActivityCallback(PassOwnPtr<GCActivityCallback>);
         JS_EXPORT_PRIVATE void setGarbageCollectionTimerEnabled(bool);
 
         JS_EXPORT_PRIVATE IncrementalSweeper* sweeper();
@@ -172,7 +172,6 @@
         void didAbandon(size_t);
 
         bool isPagedOut(double deadline);
-        void didStartVMShutdown();
         
         const JITStubRoutineSet& jitStubRoutines() { return m_jitStubRoutines; }
 
@@ -265,8 +264,8 @@
 
         DoublyLinkedList<ExecutableBase> m_compiledCode;
         
-        GCActivityCallback* m_activityCallback;
-        IncrementalSweeper* m_sweeper;
+        OwnPtr<GCActivityCallback> m_activityCallback;
+        OwnPtr<IncrementalSweeper> m_sweeper;
         Vector<MarkedBlock*> m_blockSnapshot;
     };
 

Modified: trunk/Source/_javascript_Core/heap/HeapTimer.cpp (148474 => 148475)


--- trunk/Source/_javascript_Core/heap/HeapTimer.cpp	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/heap/HeapTimer.cpp	2013-04-15 23:17:51 UTC (rev 148475)
@@ -46,12 +46,25 @@
     
 const CFTimeInterval HeapTimer::s_decade = 60 * 60 * 24 * 365 * 10;
 
+static const void* retainAPILock(const void* info)
+{
+    static_cast<JSLock*>(const_cast<void*>(info))->ref();
+    return info;
+}
+
+static void releaseAPILock(const void* info)
+{
+    static_cast<JSLock*>(const_cast<void*>(info))->deref();
+}
+
 HeapTimer::HeapTimer(JSGlobalData* globalData, CFRunLoopRef runLoop)
     : m_globalData(globalData)
     , m_runLoop(runLoop)
 {
     memset(&m_context, 0, sizeof(CFRunLoopTimerContext));
-    m_context.info = this;
+    m_context.info = &globalData->apiLock();
+    m_context.retain = retainAPILock;
+    m_context.release = releaseAPILock;
     m_timer.adoptCF(CFRunLoopTimerCreate(0, s_decade, s_decade, 0, 0, HeapTimer::timerDidFire, &m_context));
     CFRunLoopAddTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
 }
@@ -71,39 +84,32 @@
     CFRunLoopAddTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
 }
 
-void HeapTimer::invalidate()
+void HeapTimer::timerDidFire(CFRunLoopTimerRef timer, void* context)
 {
-    m_globalData = 0;
-    CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() - s_decade);
-}
+    JSLock* apiLock = static_cast<JSLock*>(context);
+    apiLock->lock();
 
-void HeapTimer::didStartVMShutdown()
-{
-    if (CFRunLoopGetCurrent() == m_runLoop.get()) {
-        invalidate();
-        delete this;
+    JSGlobalData* globalData = apiLock->globalData();
+    // The JSGlobalData has been destroyed, so we should just give up.
+    if (!globalData) {
+        apiLock->unlock();
         return;
     }
-    ASSERT(!m_globalData->apiLock().currentThreadIsHoldingLock());
-    MutexLocker locker(m_shutdownMutex);
-    invalidate();
-}
 
-void HeapTimer::timerDidFire(CFRunLoopTimerRef, void* info)
-{
-    HeapTimer* agent = static_cast<HeapTimer*>(info);
-    agent->m_shutdownMutex.lock();
-    if (!agent->m_globalData) {
-        agent->m_shutdownMutex.unlock();
-        delete agent;
-        return;
-    }
+    HeapTimer* heapTimer = 0;
+    if (globalData->heap.activityCallback()->m_timer.get() == timer)
+        heapTimer = globalData->heap.activityCallback();
+    else if (globalData->heap.sweeper()->m_timer.get() == timer)
+        heapTimer = globalData->heap.sweeper();
+    else
+        RELEASE_ASSERT_NOT_REACHED();
+
     {
-        // We don't ref here to prevent us from resurrecting the ref count of a "dead" JSGlobalData.
-        APIEntryShim shim(agent->m_globalData, APIEntryShimWithoutLock::DontRefGlobalData);
-        agent->doWork();
+        APIEntryShim shim(globalData);
+        heapTimer->doWork();
     }
-    agent->m_shutdownMutex.unlock();
+
+    apiLock->unlock();
 }
 
 #elif PLATFORM(BLACKBERRY)
@@ -134,11 +140,6 @@
 {
 }
 
-void HeapTimer::didStartVMShutdown()
-{
-    delete this;
-}
-
 #elif PLATFORM(QT)
 
 HeapTimer::HeapTimer(JSGlobalData* globalData)
@@ -153,6 +154,8 @@
 
 HeapTimer::~HeapTimer()
 {
+    QMutexLocker lock(&m_mutex);
+    m_timer.stop();
 }
 
 void HeapTimer::timerEvent(QTimerEvent*)
@@ -163,7 +166,7 @@
         return;
     }
 
-    APIEntryShim shim(m_globalData, APIEntryShimWithoutLock::DontRefGlobalData);
+    APIEntryShim shim(m_globalData);
     doWork();
 }
 
@@ -187,21 +190,6 @@
     }
 }
 
-void HeapTimer::invalidate()
-{
-    QMutexLocker lock(&m_mutex);
-    m_timer.stop();
-}
-
-void HeapTimer::didStartVMShutdown()
-{
-    invalidate();
-    if (thread() == QThread::currentThread())
-        delete this;
-    else
-        deleteLater();
-}
-
 #else
 HeapTimer::HeapTimer(JSGlobalData* globalData)
     : m_globalData(globalData)
@@ -212,11 +200,6 @@
 {
 }
 
-void HeapTimer::didStartVMShutdown()
-{
-    delete this;
-}
-
 void HeapTimer::synchronize()
 {
 }

Modified: trunk/Source/_javascript_Core/heap/HeapTimer.h (148474 => 148475)


--- trunk/Source/_javascript_Core/heap/HeapTimer.h	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/heap/HeapTimer.h	2013-04-15 23:17:51 UTC (rev 148475)
@@ -59,7 +59,6 @@
     
     virtual ~HeapTimer();
 
-    void didStartVMShutdown();
     virtual void synchronize();
     virtual void doWork() = 0;
     

Modified: trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp (148474 => 148475)


--- trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp	2013-04-15 23:17:51 UTC (rev 148475)
@@ -52,9 +52,9 @@
 {
 }
 
-IncrementalSweeper* IncrementalSweeper::create(Heap* heap)
+PassOwnPtr<IncrementalSweeper> IncrementalSweeper::create(Heap* heap)
 {
-    return new IncrementalSweeper(heap, CFRunLoopGetCurrent());
+    return adoptPtr(new IncrementalSweeper(heap, CFRunLoopGetCurrent()));
 }
 
 void IncrementalSweeper::scheduleTimer()
@@ -76,9 +76,9 @@
 {
 }
 
-IncrementalSweeper* IncrementalSweeper::create(Heap* heap)
+PassOwnPtr<IncrementalSweeper> IncrementalSweeper::create(Heap* heap)
 {
-    return new IncrementalSweeper(heap);
+    return adoptPtr(new IncrementalSweeper(heap));
 }
 
 void IncrementalSweeper::scheduleTimer()
@@ -159,9 +159,9 @@
 {
 }
 
-IncrementalSweeper* IncrementalSweeper::create(Heap* heap)
+PassOwnPtr<IncrementalSweeper> IncrementalSweeper::create(Heap* heap)
 {
-    return new IncrementalSweeper(heap->globalData());
+    return adoptPtr(new IncrementalSweeper(heap->globalData()));
 }
 
 void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>&)

Modified: trunk/Source/_javascript_Core/heap/IncrementalSweeper.h (148474 => 148475)


--- trunk/Source/_javascript_Core/heap/IncrementalSweeper.h	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/heap/IncrementalSweeper.h	2013-04-15 23:17:51 UTC (rev 148475)
@@ -39,7 +39,7 @@
 
 class IncrementalSweeper : public HeapTimer {
 public:
-    static IncrementalSweeper* create(Heap*);
+    static PassOwnPtr<IncrementalSweeper> create(Heap*);
     void startSweeping(Vector<MarkedBlock*>&);
     virtual void doWork();
     void sweepNextBlock();

Modified: trunk/Source/_javascript_Core/jsc.cpp (148474 => 148475)


--- trunk/Source/_javascript_Core/jsc.cpp	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/jsc.cpp	2013-04-15 23:17:51 UTC (rev 148475)
@@ -22,6 +22,7 @@
 
 #include "config.h"
 
+#include "APIShims.h"
 #include "ButterflyInlines.h"
 #include "BytecodeGenerator.h"
 #include "Completion.h"
@@ -760,8 +761,8 @@
     // Note that the options parsing can affect JSGlobalData creation, and thus
     // comes first.
     CommandLine options(argc, argv);
-    RefPtr<JSGlobalData> globalData = JSGlobalData::create(LargeHeap);
-    JSLockHolder lock(globalData.get());
+    JSGlobalData* globalData = JSGlobalData::create(LargeHeap).leakRef();
+    APIEntryShim shim(globalData);
     int result;
 
     if (options.m_profile && !globalData->m_perBytecodeProfiler)

Modified: trunk/Source/_javascript_Core/runtime/GCActivityCallback.h (148474 => 148475)


--- trunk/Source/_javascript_Core/runtime/GCActivityCallback.h	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/runtime/GCActivityCallback.h	2013-04-15 23:17:51 UTC (rev 148475)
@@ -70,7 +70,7 @@
 
 class DefaultGCActivityCallback : public GCActivityCallback {
 public:
-    static DefaultGCActivityCallback* create(Heap*);
+    static PassOwnPtr<DefaultGCActivityCallback> create(Heap*);
 
     DefaultGCActivityCallback(Heap*);
 
@@ -94,9 +94,9 @@
 #endif
 };
 
-inline DefaultGCActivityCallback* DefaultGCActivityCallback::create(Heap* heap)
+inline PassOwnPtr<DefaultGCActivityCallback> DefaultGCActivityCallback::create(Heap* heap)
 {
-    return new DefaultGCActivityCallback(heap);
+    return adoptPtr(new DefaultGCActivityCallback(heap));
 }
 
 }

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalData.cpp (148474 => 148475)


--- trunk/Source/_javascript_Core/runtime/JSGlobalData.cpp	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalData.cpp	2013-04-15 23:17:51 UTC (rev 148475)
@@ -134,11 +134,11 @@
 #endif // ENABLE(!ASSEMBLER)
 
 JSGlobalData::JSGlobalData(GlobalDataType globalDataType, HeapType heapType)
-    :
+    : m_apiLock(adoptRef(new JSLock(this)))
 #if ENABLE(ASSEMBLER)
-      executableAllocator(*this),
+    , executableAllocator(*this)
 #endif
-      heap(this, heapType)
+    , heap(this, heapType)
     , globalDataType(globalDataType)
     , clientData(0)
     , topCallFrame(CallFrame::noCaller())
@@ -270,8 +270,9 @@
     // Clear this first to ensure that nobody tries to remove themselves from it.
     m_perBytecodeProfiler.clear();
     
-    ASSERT(!m_apiLock.currentThreadIsHoldingLock());
-    heap.didStartVMShutdown();
+    ASSERT(m_apiLock->currentThreadIsHoldingLock());
+    m_apiLock->willDestroyGlobalData(this);
+    heap.lastChanceToFinalize();
 
     delete interpreter;
 #ifndef NDEBUG

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalData.h (148474 => 148475)


--- trunk/Source/_javascript_Core/runtime/JSGlobalData.h	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalData.h	2013-04-15 23:17:51 UTC (rev 148475)
@@ -191,7 +191,7 @@
         void makeUsableFromMultipleThreads() { heap.machineThreads().makeUsableFromMultipleThreads(); }
 
     private:
-        JSLock m_apiLock;
+        RefPtr<JSLock> m_apiLock;
 
     public:
 #if ENABLE(ASSEMBLER)
@@ -468,7 +468,7 @@
             }
         }
 
-        JSLock& apiLock() { return m_apiLock; }
+        JSLock& apiLock() { return *m_apiLock; }
         CodeCache* codeCache() { return m_codeCache.get(); }
 
         JS_EXPORT_PRIVATE void discardAllCode();

Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (148474 => 148475)


--- trunk/Source/_javascript_Core/runtime/JSLock.cpp	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp	2013-04-15 23:17:51 UTC (rev 148475)
@@ -53,30 +53,38 @@
 JSLockHolder::JSLockHolder(ExecState* exec)
     : m_globalData(&exec->globalData())
 {
-    m_globalData->apiLock().lock();
+    init();
 }
 
 JSLockHolder::JSLockHolder(JSGlobalData* globalData)
     : m_globalData(globalData)
 {
-    m_globalData->apiLock().lock();
+    init();
 }
 
 JSLockHolder::JSLockHolder(JSGlobalData& globalData)
     : m_globalData(&globalData)
 {
+    init();
+}
+
+void JSLockHolder::init()
+{
     m_globalData->apiLock().lock();
 }
 
 JSLockHolder::~JSLockHolder()
 {
-    m_globalData->apiLock().unlock();
+    RefPtr<JSLock> apiLock(&m_globalData->apiLock());
+    m_globalData.clear();
+    apiLock->unlock();
 }
 
-JSLock::JSLock()
+JSLock::JSLock(JSGlobalData* globalData)
     : m_ownerThread(0)
     , m_lockCount(0)
     , m_lockDropDepth(0)
+    , m_globalData(globalData)
 {
     m_spinLock.Init();
 }
@@ -85,6 +93,12 @@
 {
 }
 
+void JSLock::willDestroyGlobalData(JSGlobalData* globalData)
+{
+    ASSERT_UNUSED(globalData, m_globalData == globalData);
+    m_globalData = 0;
+}
+
 void JSLock::lock()
 {
     ThreadIdentifier currentThread = WTF::currentThread();

Modified: trunk/Source/_javascript_Core/runtime/JSLock.h (148474 => 148475)


--- trunk/Source/_javascript_Core/runtime/JSLock.h	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/runtime/JSLock.h	2013-04-15 23:17:51 UTC (rev 148475)
@@ -72,13 +72,15 @@
 
         JS_EXPORT_PRIVATE ~JSLockHolder();
     private:
+        void init();
+
         RefPtr<JSGlobalData> m_globalData;
     };
 
-    class JSLock {
+    class JSLock : public ThreadSafeRefCounted<JSLock> {
         WTF_MAKE_NONCOPYABLE(JSLock);
     public:
-        JSLock();
+        JSLock(JSGlobalData*);
         JS_EXPORT_PRIVATE ~JSLock();
 
         JS_EXPORT_PRIVATE void lock();
@@ -89,12 +91,16 @@
         static void lock(JSGlobalData&);
         static void unlock(JSGlobalData&);
 
+        JSGlobalData* globalData() { return m_globalData; }
+
         JS_EXPORT_PRIVATE bool currentThreadIsHoldingLock();
 
         unsigned dropAllLocks();
         unsigned dropAllLocksUnconditionally();
         void grabAllLocks(unsigned lockCount);
 
+        void willDestroyGlobalData(JSGlobalData*);
+
         class DropAllLocks {
             WTF_MAKE_NONCOPYABLE(DropAllLocks);
         public:
@@ -113,6 +119,7 @@
         ThreadIdentifier m_ownerThread;
         intptr_t m_lockCount;
         unsigned m_lockDropDepth;
+        JSGlobalData* m_globalData;
     };
 
 } // namespace

Modified: trunk/Source/_javascript_Core/testRegExp.cpp (148474 => 148475)


--- trunk/Source/_javascript_Core/testRegExp.cpp	2013-04-15 23:04:15 UTC (rev 148474)
+++ trunk/Source/_javascript_Core/testRegExp.cpp	2013-04-15 23:17:51 UTC (rev 148475)
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "RegExp.h"
 
+#include "APIShims.h"
 #include <wtf/CurrentTime.h>
 #include "InitializeThreading.h"
 #include "JSGlobalObject.h"
@@ -499,8 +500,8 @@
 
 int realMain(int argc, char** argv)
 {
-    RefPtr<JSGlobalData> globalData = JSGlobalData::create(LargeHeap);
-    JSLockHolder lock(globalData.get());
+    JSGlobalData* globalData = JSGlobalData::create(LargeHeap).leakRef();
+    APIEntryShim shim(globalData);
 
     CommandLine options;
     parseArguments(argc, argv, options);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to