JSRunLoopTimer may run part of a member function after it's destroyed
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2018 23:57:03 +0000 (23:57 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2018 23:57:03 +0000 (23:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188426

Reviewed by Mark Lam.

Source/JavaScriptCore:

When I was reading the JSRunLoopTimer code, I noticed that it is possible
to end up running timer code after the class had been destroyed.

The issue I spotted was in this function:
```
void JSRunLoopTimer::timerDidFire()
{
    JSLock* apiLock = m_apiLock.get();
    if (!apiLock) {
        // Likely a buggy usage: the timer fired while JSRunLoopTimer was being destroyed.
        return;
    }
    // HERE
    std::lock_guard<JSLock> lock(*apiLock);
    RefPtr<VM> vm = apiLock->vm();
    if (!vm) {
        // The VM has been destroyed, so we should just give up.
        return;
    }

    doWork();
}
```

Look at the comment 'HERE'. Let's say that the timer callback thread gets context
switched before grabbing the API lock. Then, some other thread destroys the VM.
And let's say that the VM owns (perhaps transitively) this timer. Then, the
timer would run code and access member variables after it was destroyed.

This patch fixes this issue by introducing a new timer manager class.
This class manages timers on a per VM basis. When a timer is scheduled,
this class refs the timer. It also calls the timer callback while actively
maintaining a +1 ref to it. So, it's no longer possible to call the timer
callback after the timer has been destroyed. However, calling a timer callback
can still race with the VM being destroyed. We continue to detect this case and
bail out of the callback early.

This patch also removes a lot of duplicate code between GCActivityCallback
and JSRunLoopTimer.

* heap/EdenGCActivityCallback.cpp:
(JSC::EdenGCActivityCallback::doCollection):
(JSC::EdenGCActivityCallback::lastGCLength):
(JSC::EdenGCActivityCallback::deathRate):
* heap/EdenGCActivityCallback.h:
* heap/FullGCActivityCallback.cpp:
(JSC::FullGCActivityCallback::doCollection):
(JSC::FullGCActivityCallback::lastGCLength):
(JSC::FullGCActivityCallback::deathRate):
* heap/FullGCActivityCallback.h:
* heap/GCActivityCallback.cpp:
(JSC::GCActivityCallback::doWork):
(JSC::GCActivityCallback::scheduleTimer):
(JSC::GCActivityCallback::didAllocate):
(JSC::GCActivityCallback::willCollect):
(JSC::GCActivityCallback::cancel):
(JSC::GCActivityCallback::cancelTimer): Deleted.
(JSC::GCActivityCallback::nextFireTime): Deleted.
* heap/GCActivityCallback.h:
* heap/Heap.cpp:
(JSC::Heap::reportAbandonedObjectGraph):
(JSC::Heap::notifyIncrementalSweeper):
(JSC::Heap::updateAllocationLimits):
(JSC::Heap::didAllocate):
* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::scheduleTimer):
(JSC::IncrementalSweeper::doWork):
(JSC::IncrementalSweeper::doSweep):
(JSC::IncrementalSweeper::sweepNextBlock):
(JSC::IncrementalSweeper::startSweeping):
(JSC::IncrementalSweeper::stopSweeping):
* heap/IncrementalSweeper.h:
* heap/StopIfNecessaryTimer.cpp:
(JSC::StopIfNecessaryTimer::doWork):
(JSC::StopIfNecessaryTimer::scheduleSoon):
* heap/StopIfNecessaryTimer.h:
* runtime/JSRunLoopTimer.cpp:
(JSC::epochTime):
(JSC::JSRunLoopTimer::Manager::timerDidFireCallback):
(JSC::JSRunLoopTimer::Manager::PerVMData::setRunLoop):
(JSC::JSRunLoopTimer::Manager::PerVMData::PerVMData):
(JSC::JSRunLoopTimer::Manager::PerVMData::~PerVMData):
(JSC::JSRunLoopTimer::Manager::timerDidFire):
(JSC::JSRunLoopTimer::Manager::shared):
(JSC::JSRunLoopTimer::Manager::registerVM):
(JSC::JSRunLoopTimer::Manager::unregisterVM):
(JSC::JSRunLoopTimer::Manager::scheduleTimer):
(JSC::JSRunLoopTimer::Manager::cancelTimer):
(JSC::JSRunLoopTimer::Manager::timeUntilFire):
(JSC::JSRunLoopTimer::Manager::didChangeRunLoop):
(JSC::JSRunLoopTimer::timerDidFire):
(JSC::JSRunLoopTimer::JSRunLoopTimer):
(JSC::JSRunLoopTimer::timeUntilFire):
(JSC::JSRunLoopTimer::setTimeUntilFire):
(JSC::JSRunLoopTimer::cancelTimer):
(JSC::JSRunLoopTimer::setRunLoop): Deleted.
(JSC::JSRunLoopTimer::timerDidFireCallback): Deleted.
(JSC::JSRunLoopTimer::scheduleTimer): Deleted.
* runtime/JSRunLoopTimer.h:
(JSC::JSRunLoopTimer::Manager::PerVMData::PerVMData):
* runtime/PromiseDeferredTimer.cpp:
(JSC::PromiseDeferredTimer::doWork):
(JSC::PromiseDeferredTimer::runRunLoop):
(JSC::PromiseDeferredTimer::addPendingPromise):
(JSC::PromiseDeferredTimer::hasPendingPromise):
(JSC::PromiseDeferredTimer::hasDependancyInPendingPromise):
(JSC::PromiseDeferredTimer::cancelPendingPromise):
(JSC::PromiseDeferredTimer::scheduleWorkSoon):
* runtime/PromiseDeferredTimer.h:
* runtime/VM.cpp:
(JSC::VM::VM):
(JSC::VM::~VM):
(JSC::VM::setRunLoop):
(JSC::VM::registerRunLoopTimer): Deleted.
(JSC::VM::unregisterRunLoopTimer): Deleted.
* runtime/VM.h:
(JSC::VM::runLoop const):
* wasm/js/WebAssemblyPrototype.cpp:
(JSC::webAssemblyModuleValidateAsyncInternal):
(JSC::instantiate):
(JSC::compileAndInstantiate):
(JSC::webAssemblyModuleInstantinateAsyncInternal):
(JSC::webAssemblyCompileStreamingInternal):
(JSC::webAssemblyInstantiateStreamingInternal):

Source/WebCore:

* page/cocoa/ResourceUsageThreadCocoa.mm:
(WebCore::ResourceUsageThread::platformThreadBody):
* page/linux/ResourceUsageThreadLinux.cpp:
(WebCore::ResourceUsageThread::platformThreadBody):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235261 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp
Source/JavaScriptCore/heap/EdenGCActivityCallback.h
Source/JavaScriptCore/heap/FullGCActivityCallback.cpp
Source/JavaScriptCore/heap/FullGCActivityCallback.h
Source/JavaScriptCore/heap/GCActivityCallback.cpp
Source/JavaScriptCore/heap/GCActivityCallback.h
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/IncrementalSweeper.cpp
Source/JavaScriptCore/heap/IncrementalSweeper.h
Source/JavaScriptCore/heap/StopIfNecessaryTimer.cpp
Source/JavaScriptCore/heap/StopIfNecessaryTimer.h
Source/JavaScriptCore/runtime/JSRunLoopTimer.cpp
Source/JavaScriptCore/runtime/JSRunLoopTimer.h
Source/JavaScriptCore/runtime/PromiseDeferredTimer.cpp
Source/JavaScriptCore/runtime/PromiseDeferredTimer.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp
Source/WebCore/ChangeLog
Source/WebCore/page/cocoa/ResourceUsageThreadCocoa.mm
Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp

index 59ec94c..5fa2c2a 100644 (file)
@@ -1,3 +1,135 @@
+2018-08-23  Saam barati  <sbarati@apple.com>
+
+        JSRunLoopTimer may run part of a member function after it's destroyed
+        https://bugs.webkit.org/show_bug.cgi?id=188426
+
+        Reviewed by Mark Lam.
+
+        When I was reading the JSRunLoopTimer code, I noticed that it is possible
+        to end up running timer code after the class had been destroyed.
+        
+        The issue I spotted was in this function:
+        ```
+        void JSRunLoopTimer::timerDidFire()
+        {
+            JSLock* apiLock = m_apiLock.get();
+            if (!apiLock) {
+                // Likely a buggy usage: the timer fired while JSRunLoopTimer was being destroyed.
+                return;
+            }
+            // HERE
+            std::lock_guard<JSLock> lock(*apiLock);
+            RefPtr<VM> vm = apiLock->vm();
+            if (!vm) {
+                // The VM has been destroyed, so we should just give up.
+                return;
+            }
+        
+            doWork();
+        }
+        ```
+        
+        Look at the comment 'HERE'. Let's say that the timer callback thread gets context
+        switched before grabbing the API lock. Then, some other thread destroys the VM.
+        And let's say that the VM owns (perhaps transitively) this timer. Then, the
+        timer would run code and access member variables after it was destroyed.
+        
+        This patch fixes this issue by introducing a new timer manager class. 
+        This class manages timers on a per VM basis. When a timer is scheduled,
+        this class refs the timer. It also calls the timer callback while actively
+        maintaining a +1 ref to it. So, it's no longer possible to call the timer
+        callback after the timer has been destroyed. However, calling a timer callback
+        can still race with the VM being destroyed. We continue to detect this case and
+        bail out of the callback early.
+        
+        This patch also removes a lot of duplicate code between GCActivityCallback
+        and JSRunLoopTimer.
+
+        * heap/EdenGCActivityCallback.cpp:
+        (JSC::EdenGCActivityCallback::doCollection):
+        (JSC::EdenGCActivityCallback::lastGCLength):
+        (JSC::EdenGCActivityCallback::deathRate):
+        * heap/EdenGCActivityCallback.h:
+        * heap/FullGCActivityCallback.cpp:
+        (JSC::FullGCActivityCallback::doCollection):
+        (JSC::FullGCActivityCallback::lastGCLength):
+        (JSC::FullGCActivityCallback::deathRate):
+        * heap/FullGCActivityCallback.h:
+        * heap/GCActivityCallback.cpp:
+        (JSC::GCActivityCallback::doWork):
+        (JSC::GCActivityCallback::scheduleTimer):
+        (JSC::GCActivityCallback::didAllocate):
+        (JSC::GCActivityCallback::willCollect):
+        (JSC::GCActivityCallback::cancel):
+        (JSC::GCActivityCallback::cancelTimer): Deleted.
+        (JSC::GCActivityCallback::nextFireTime): Deleted.
+        * heap/GCActivityCallback.h:
+        * heap/Heap.cpp:
+        (JSC::Heap::reportAbandonedObjectGraph):
+        (JSC::Heap::notifyIncrementalSweeper):
+        (JSC::Heap::updateAllocationLimits):
+        (JSC::Heap::didAllocate):
+        * heap/IncrementalSweeper.cpp:
+        (JSC::IncrementalSweeper::scheduleTimer):
+        (JSC::IncrementalSweeper::doWork):
+        (JSC::IncrementalSweeper::doSweep):
+        (JSC::IncrementalSweeper::sweepNextBlock):
+        (JSC::IncrementalSweeper::startSweeping):
+        (JSC::IncrementalSweeper::stopSweeping):
+        * heap/IncrementalSweeper.h:
+        * heap/StopIfNecessaryTimer.cpp:
+        (JSC::StopIfNecessaryTimer::doWork):
+        (JSC::StopIfNecessaryTimer::scheduleSoon):
+        * heap/StopIfNecessaryTimer.h:
+        * runtime/JSRunLoopTimer.cpp:
+        (JSC::epochTime):
+        (JSC::JSRunLoopTimer::Manager::timerDidFireCallback):
+        (JSC::JSRunLoopTimer::Manager::PerVMData::setRunLoop):
+        (JSC::JSRunLoopTimer::Manager::PerVMData::PerVMData):
+        (JSC::JSRunLoopTimer::Manager::PerVMData::~PerVMData):
+        (JSC::JSRunLoopTimer::Manager::timerDidFire):
+        (JSC::JSRunLoopTimer::Manager::shared):
+        (JSC::JSRunLoopTimer::Manager::registerVM):
+        (JSC::JSRunLoopTimer::Manager::unregisterVM):
+        (JSC::JSRunLoopTimer::Manager::scheduleTimer):
+        (JSC::JSRunLoopTimer::Manager::cancelTimer):
+        (JSC::JSRunLoopTimer::Manager::timeUntilFire):
+        (JSC::JSRunLoopTimer::Manager::didChangeRunLoop):
+        (JSC::JSRunLoopTimer::timerDidFire):
+        (JSC::JSRunLoopTimer::JSRunLoopTimer):
+        (JSC::JSRunLoopTimer::timeUntilFire):
+        (JSC::JSRunLoopTimer::setTimeUntilFire):
+        (JSC::JSRunLoopTimer::cancelTimer):
+        (JSC::JSRunLoopTimer::setRunLoop): Deleted.
+        (JSC::JSRunLoopTimer::timerDidFireCallback): Deleted.
+        (JSC::JSRunLoopTimer::scheduleTimer): Deleted.
+        * runtime/JSRunLoopTimer.h:
+        (JSC::JSRunLoopTimer::Manager::PerVMData::PerVMData):
+        * runtime/PromiseDeferredTimer.cpp:
+        (JSC::PromiseDeferredTimer::doWork):
+        (JSC::PromiseDeferredTimer::runRunLoop):
+        (JSC::PromiseDeferredTimer::addPendingPromise):
+        (JSC::PromiseDeferredTimer::hasPendingPromise):
+        (JSC::PromiseDeferredTimer::hasDependancyInPendingPromise):
+        (JSC::PromiseDeferredTimer::cancelPendingPromise):
+        (JSC::PromiseDeferredTimer::scheduleWorkSoon):
+        * runtime/PromiseDeferredTimer.h:
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+        (JSC::VM::~VM):
+        (JSC::VM::setRunLoop):
+        (JSC::VM::registerRunLoopTimer): Deleted.
+        (JSC::VM::unregisterRunLoopTimer): Deleted.
+        * runtime/VM.h:
+        (JSC::VM::runLoop const):
+        * wasm/js/WebAssemblyPrototype.cpp:
+        (JSC::webAssemblyModuleValidateAsyncInternal):
+        (JSC::instantiate):
+        (JSC::compileAndInstantiate):
+        (JSC::webAssemblyModuleInstantinateAsyncInternal):
+        (JSC::webAssemblyCompileStreamingInternal):
+        (JSC::webAssemblyInstantiateStreamingInternal):
+
 2018-08-23  Mark Lam  <mark.lam@apple.com>
 
         Move vmEntryGlobalObject() to VM from CallFrame.
index b6bde3d..0ca5014 100644 (file)
@@ -36,21 +36,20 @@ EdenGCActivityCallback::EdenGCActivityCallback(Heap* heap)
 {
 }
 
-void EdenGCActivityCallback::doCollection()
+void EdenGCActivityCallback::doCollection(VM& vm)
 {
-    m_vm->heap.collectAsync(CollectionScope::Eden);
+    vm.heap.collectAsync(CollectionScope::Eden);
 }
 
-Seconds EdenGCActivityCallback::lastGCLength()
+Seconds EdenGCActivityCallback::lastGCLength(Heap& heap)
 {
-    return m_vm->heap.lastEdenGCLength();
+    return heap.lastEdenGCLength();
 }
 
-double EdenGCActivityCallback::deathRate()
+double EdenGCActivityCallback::deathRate(Heap& heap)
 {
-    Heap* heap = &m_vm->heap;
-    size_t sizeBefore = heap->sizeBeforeLastEdenCollection();
-    size_t sizeAfter = heap->sizeAfterLastEdenCollection();
+    size_t sizeBefore = heap.sizeBeforeLastEdenCollection();
+    size_t sizeAfter = heap.sizeAfterLastEdenCollection();
     if (!sizeBefore)
         return 1.0;
     if (sizeAfter > sizeBefore) {
index 87482bd..52e0a28 100644 (file)
@@ -33,12 +33,12 @@ class JS_EXPORT_PRIVATE EdenGCActivityCallback : public GCActivityCallback {
 public:
     EdenGCActivityCallback(Heap*);
 
-    void doCollection() override;
+    void doCollection(VM&) override;
 
 protected:
-    Seconds lastGCLength() override;
+    Seconds lastGCLength(Heap&) override;
     double gcTimeSlice(size_t bytes) override;
-    double deathRate() override;
+    double deathRate(Heap&) override;
 };
 
 inline RefPtr<GCActivityCallback> GCActivityCallback::createEdenTimer(Heap* heap)
index d998a29..c9ef7fe 100644 (file)
@@ -39,9 +39,9 @@ FullGCActivityCallback::FullGCActivityCallback(Heap* heap)
 {
 }
 
-void FullGCActivityCallback::doCollection()
+void FullGCActivityCallback::doCollection(VM& vm)
 {
-    Heap& heap = m_vm->heap;
+    Heap& heap = vm.heap;
     m_didGCRecently = false;
 
 #if !PLATFORM(IOS)
@@ -56,16 +56,15 @@ void FullGCActivityCallback::doCollection()
     heap.collectAsync(CollectionScope::Full);
 }
 
-Seconds FullGCActivityCallback::lastGCLength()
+Seconds FullGCActivityCallback::lastGCLength(Heap& heap)
 {
-    return m_vm->heap.lastFullGCLength();
+    return heap.lastFullGCLength();
 }
 
-double FullGCActivityCallback::deathRate()
+double FullGCActivityCallback::deathRate(Heap& heap)
 {
-    Heap* heap = &m_vm->heap;
-    size_t sizeBefore = heap->sizeBeforeLastFullCollection();
-    size_t sizeAfter = heap->sizeAfterLastFullCollection();
+    size_t sizeBefore = heap.sizeBeforeLastFullCollection();
+    size_t sizeAfter = heap.sizeAfterLastFullCollection();
     if (!sizeBefore)
         return 1.0;
     if (sizeAfter > sizeBefore) {
index da89c7d..5ce034a 100644 (file)
@@ -33,15 +33,15 @@ class JS_EXPORT_PRIVATE FullGCActivityCallback : public GCActivityCallback {
 public:
     FullGCActivityCallback(Heap*);
 
-    void doCollection() override;
+    void doCollection(VM&) override;
 
     bool didGCRecently() const { return m_didGCRecently; }
     void setDidGCRecently() { m_didGCRecently = true; }
 
 protected:
-    Seconds lastGCLength() override;
+    Seconds lastGCLength(Heap&) override;
     double gcTimeSlice(size_t bytes) override;
-    double deathRate() override;
+    double deathRate(Heap&) override;
 
     bool m_didGCRecently { false };
 };
index 007fdd8..d9df4ea 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -45,83 +45,52 @@ GCActivityCallback::GCActivityCallback(Heap* heap)
 {
 }
 
-void GCActivityCallback::doWork()
+void GCActivityCallback::doWork(VM& vm)
 {
-    Heap* heap = &m_vm->heap;
     if (!isEnabled())
         return;
     
-    JSLockHolder locker(m_vm);
-    if (heap->isDeferred()) {
+    ASSERT(vm.currentThreadIsHoldingAPILock());
+    Heap& heap = vm.heap;
+    if (heap.isDeferred()) {
         scheduleTimer(0_s);
         return;
     }
 
-    doCollection();
+    doCollection(vm);
 }
 
-#if USE(CF)
 void GCActivityCallback::scheduleTimer(Seconds newDelay)
 {
     if (newDelay * timerSlop > m_delay)
         return;
     Seconds delta = m_delay - newDelay;
     m_delay = newDelay;
-    CFRunLoopTimerSetNextFireDate(m_timer.get(), CFRunLoopTimerGetNextFireDate(m_timer.get()) - delta.seconds());
+    if (auto timeUntilFire = this->timeUntilFire())
+        setTimeUntilFire(*timeUntilFire - delta);
+    else
+        setTimeUntilFire(newDelay);
 }
 
-void GCActivityCallback::cancelTimer()
-{
-    m_delay = s_decade;
-    CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + s_decade.seconds());
-}
-
-MonotonicTime GCActivityCallback::nextFireTime()
-{
-    return MonotonicTime::now() + Seconds(CFRunLoopTimerGetNextFireDate(m_timer.get()) - CFAbsoluteTimeGetCurrent());
-}
-#else
-void GCActivityCallback::scheduleTimer(Seconds newDelay)
-{
-    if (newDelay * timerSlop > m_delay)
-        return;
-    Seconds delta = m_delay - newDelay;
-    m_delay = newDelay;
-
-    Seconds secondsUntilFire = m_timer.secondsUntilFire();
-    m_timer.startOneShot(std::max<Seconds>(secondsUntilFire - delta, 0_s));
-}
-
-void GCActivityCallback::cancelTimer()
-{
-    m_delay = s_decade;
-    m_timer.startOneShot(s_decade);
-}
-
-MonotonicTime GCActivityCallback::nextFireTime()
-{
-    return MonotonicTime::now() + m_timer.secondsUntilFire();
-}
-#endif
-
-void GCActivityCallback::didAllocate(size_t bytes)
+void GCActivityCallback::didAllocate(Heap& heap, size_t bytes)
 {
     // The first byte allocated in an allocation cycle will report 0 bytes to didAllocate. 
     // We pretend it's one byte so that we don't ignore this allocation entirely.
     if (!bytes)
         bytes = 1;
-    double bytesExpectedToReclaim = static_cast<double>(bytes) * deathRate();
-    Seconds newDelay = lastGCLength() / gcTimeSlice(bytesExpectedToReclaim);
+    double bytesExpectedToReclaim = static_cast<double>(bytes) * deathRate(heap);
+    Seconds newDelay = lastGCLength(heap) / gcTimeSlice(bytesExpectedToReclaim);
     scheduleTimer(newDelay);
 }
 
 void GCActivityCallback::willCollect()
 {
-    cancelTimer();
+    cancel();
 }
 
 void GCActivityCallback::cancel()
 {
+    m_delay = s_decade;
     cancelTimer();
 }
 
index 4b40676..9dfdd0f 100644 (file)
@@ -48,24 +48,22 @@ public:
 
     GCActivityCallback(Heap*);
 
-    void doWork() override;
+    void doWork(VM&) override;
 
-    virtual void doCollection() = 0;
+    virtual void doCollection(VM&) = 0;
 
-    virtual void didAllocate(size_t);
-    virtual void willCollect();
-    virtual void cancel();
+    void didAllocate(Heap&, size_t);
+    void willCollect();
+    void cancel();
     bool isEnabled() const { return m_enabled; }
     void setEnabled(bool enabled) { m_enabled = enabled; }
 
     static bool s_shouldCreateGCTimer;
 
-    MonotonicTime nextFireTime();
-
 protected:
-    virtual Seconds lastGCLength() = 0;
+    virtual Seconds lastGCLength(Heap&) = 0;
     virtual double gcTimeSlice(size_t bytes) = 0;
-    virtual double deathRate() = 0;
+    virtual double deathRate(Heap&) = 0;
 
     GCActivityCallback(VM* vm)
         : Base(vm)
@@ -77,7 +75,6 @@ protected:
     bool m_enabled;
 
 protected:
-    void cancelTimer();
     void scheduleTimer(Seconds);
 
 private:
index c782fcb..f84f7e6 100644 (file)
@@ -539,7 +539,7 @@ void Heap::reportAbandonedObjectGraph()
     // be more profitable. Since allocation is the trigger for collection, 
     // we hasten the next collection by pretending that we've allocated more memory. 
     if (m_fullActivityCallback) {
-        m_fullActivityCallback->didAllocate(
+        m_fullActivityCallback->didAllocate(*this,
             m_sizeAfterLastCollect - m_sizeAfterLastFullCollect + m_bytesAllocatedThisCycle + m_bytesAbandonedSinceLastFullCollect);
     }
     m_bytesAbandonedSinceLastFullCollect += abandonedBytes;
@@ -2194,7 +2194,7 @@ void Heap::notifyIncrementalSweeper()
             m_indexOfNextLogicallyEmptyWeakBlockToSweep = 0;
     }
 
-    m_sweeper->startSweeping();
+    m_sweeper->startSweeping(*this);
 }
 
 void Heap::updateAllocationLimits()
@@ -2273,7 +2273,7 @@ void Heap::updateAllocationLimits()
             dataLog("Eden: maxEdenSize = ", m_maxEdenSize, "\n");
         if (m_fullActivityCallback) {
             ASSERT(currentHeapSize >= m_sizeAfterLastFullCollect);
-            m_fullActivityCallback->didAllocate(currentHeapSize - m_sizeAfterLastFullCollect);
+            m_fullActivityCallback->didAllocate(*this, currentHeapSize - m_sizeAfterLastFullCollect);
         }
     }
 
@@ -2354,7 +2354,7 @@ void Heap::setGarbageCollectionTimerEnabled(bool enable)
 void Heap::didAllocate(size_t bytes)
 {
     if (m_edenActivityCallback)
-        m_edenActivityCallback->didAllocate(m_bytesAllocatedThisCycle + m_bytesAbandonedSinceLastFullCollect);
+        m_edenActivityCallback->didAllocate(*this, m_bytesAllocatedThisCycle + m_bytesAbandonedSinceLastFullCollect);
     m_bytesAllocatedThisCycle += bytes;
     performIncrement(bytes);
 }
index e5e49dd..22d8393 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 
 namespace JSC {
 
-static const Seconds sweepTimeSlice = 10_ms; // seconds
+static const Seconds sweepTimeSlice = 10_ms;
 static const double sweepTimeTotal = .10;
 static const double sweepTimeMultiplier = 1.0 / sweepTimeTotal;
 
 void IncrementalSweeper::scheduleTimer()
 {
-    Base::scheduleTimer(sweepTimeSlice * sweepTimeMultiplier);
+    setTimeUntilFire(sweepTimeSlice * sweepTimeMultiplier);
 }
 
 IncrementalSweeper::IncrementalSweeper(Heap* heap)
@@ -49,14 +49,14 @@ IncrementalSweeper::IncrementalSweeper(Heap* heap)
 {
 }
 
-void IncrementalSweeper::doWork()
+void IncrementalSweeper::doWork(VM& vm)
 {
-    doSweep(MonotonicTime::now());
+    doSweep(vm, MonotonicTime::now());
 }
 
-void IncrementalSweeper::doSweep(MonotonicTime sweepBeginTime)
+void IncrementalSweeper::doSweep(VM& vm, MonotonicTime sweepBeginTime)
 {
-    while (sweepNextBlock()) {
+    while (sweepNextBlock(vm)) {
         Seconds elapsedTime = MonotonicTime::now() - sweepBeginTime;
         if (elapsedTime < sweepTimeSlice)
             continue;
@@ -72,9 +72,9 @@ void IncrementalSweeper::doSweep(MonotonicTime sweepBeginTime)
     cancelTimer();
 }
 
-bool IncrementalSweeper::sweepNextBlock()
+bool IncrementalSweeper::sweepNextBlock(VM& vm)
 {
-    m_vm->heap.stopIfNecessary();
+    vm.heap.stopIfNecessary();
 
     MarkedBlock::Handle* block = nullptr;
     
@@ -85,26 +85,25 @@ bool IncrementalSweeper::sweepNextBlock()
     }
     
     if (block) {
-        DeferGCForAWhile deferGC(m_vm->heap);
+        DeferGCForAWhile deferGC(vm.heap);
         block->sweep(nullptr);
-        m_vm->heap.objectSpace().freeOrShrinkBlock(block);
+        vm.heap.objectSpace().freeOrShrinkBlock(block);
         return true;
     }
 
-    return m_vm->heap.sweepNextLogicallyEmptyWeakBlock();
+    return vm.heap.sweepNextLogicallyEmptyWeakBlock();
 }
 
-void IncrementalSweeper::startSweeping()
+void IncrementalSweeper::startSweeping(Heap& heap)
 {
     scheduleTimer();
-    m_currentDirectory = m_vm->heap.objectSpace().firstDirectory();
+    m_currentDirectory = heap.objectSpace().firstDirectory();
 }
 
 void IncrementalSweeper::stopSweeping()
 {
     m_currentDirectory = nullptr;
-    if (m_vm)
-        cancelTimer();
+    cancelTimer();
 }
 
 } // namespace JSC
index abd8e69..12646de 100644 (file)
@@ -37,15 +37,15 @@ public:
     using Base = JSRunLoopTimer;
     JS_EXPORT_PRIVATE explicit IncrementalSweeper(Heap*);
 
-    JS_EXPORT_PRIVATE void startSweeping();
+    JS_EXPORT_PRIVATE void startSweeping(Heap&);
     void freeFastMallocMemoryAfterSweeping() { m_shouldFreeFastMallocMemoryAfterSweeping = true; }
 
-    JS_EXPORT_PRIVATE void doWork() override;
-    bool sweepNextBlock();
-    JS_EXPORT_PRIVATE void stopSweeping();
+    void doWork(VM&) override;
+    void stopSweeping();
 
 private:
-    void doSweep(MonotonicTime startTime);
+    bool sweepNextBlock(VM&);
+    void doSweep(VM&, MonotonicTime startTime);
     void scheduleTimer();
     
     BlockDirectory* m_currentDirectory;
index 6239610..450d8d8 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -35,11 +35,11 @@ StopIfNecessaryTimer::StopIfNecessaryTimer(VM* vm)
 {
 }
 
-void StopIfNecessaryTimer::doWork()
+void StopIfNecessaryTimer::doWork(VM& vm)
 {
     cancelTimer();
     WTF::storeStoreFence();
-    m_vm->heap.stopIfNecessary();
+    vm.heap.stopIfNecessary();
 }
 
 void StopIfNecessaryTimer::scheduleSoon()
@@ -48,7 +48,7 @@ void StopIfNecessaryTimer::scheduleSoon()
         WTF::loadLoadFence();
         return;
     }
-    scheduleTimer(0_s);
+    setTimeUntilFire(0_s);
 }
 
 } // namespace JSC
index e1836a1..1f13373 100644 (file)
@@ -36,7 +36,7 @@ public:
     using Base = JSRunLoopTimer;
     explicit StopIfNecessaryTimer(VM*);
     
-    void doWork() override;
+    void doWork(VM&) override;
     
     void scheduleSoon();
 };
index 89edb68..a56e462 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 #include "config.h"
 #include "JSRunLoopTimer.h"
 
-#include "GCActivityCallback.h"
 #include "IncrementalSweeper.h"
 #include "JSCInlines.h"
 #include "JSObject.h"
 #include "JSString.h"
 
 #include <wtf/MainThread.h>
+#include <wtf/NoTailCalls.h>
 #include <wtf/Threading.h>
 
 #if USE(GLIB_EVENT_LOOP)
@@ -46,107 +46,288 @@ namespace JSC {
 
 const Seconds JSRunLoopTimer::s_decade { 60 * 60 * 24 * 365 * 10 };
 
-void JSRunLoopTimer::timerDidFire()
+static inline JSRunLoopTimer::Manager::EpochTime epochTime(Seconds delay)
 {
-    JSLock* apiLock = m_apiLock.get();
-    if (!apiLock) {
-        // Likely a buggy usage: the timer fired while JSRunLoopTimer was being destroyed.
-        return;
-    }
+#if USE(CF)
+    return Seconds { CFAbsoluteTimeGetCurrent() + delay.value() };
+#else
+    return MonotonicTime::now().secondsSinceEpoch() + delay;
+#endif
+}
 
-    std::lock_guard<JSLock> lock(*apiLock);
-    RefPtr<VM> vm = apiLock->vm();
-    if (!vm) {
-        // The VM has been destroyed, so we should just give up.
-        return;
+#if USE(CF)
+void JSRunLoopTimer::Manager::timerDidFireCallback(CFRunLoopTimerRef, void* contextPtr)
+{
+    static_cast<JSRunLoopTimer::Manager*>(contextPtr)->timerDidFire();
+}
+
+void JSRunLoopTimer::Manager::PerVMData::setRunLoop(Manager* manager, CFRunLoopRef newRunLoop)
+{
+    if (runLoop) {
+        CFRunLoopRemoveTimer(runLoop.get(), timer.get(), kCFRunLoopCommonModes);
+        CFRunLoopTimerInvalidate(timer.get());
+        runLoop.clear();
+        timer.clear();
     }
 
-    doWork();
+    if (newRunLoop) {
+        runLoop = newRunLoop;
+        memset(&context, 0, sizeof(CFRunLoopTimerContext));
+        RELEASE_ASSERT(manager);
+        context.info = manager;
+        timer = adoptCF(CFRunLoopTimerCreate(kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + s_decade.seconds(), CFAbsoluteTimeGetCurrent() + s_decade.seconds(), 0, 0, JSRunLoopTimer::Manager::timerDidFireCallback, &context));
+        CFRunLoopAddTimer(runLoop.get(), timer.get(), kCFRunLoopCommonModes);
+
+        EpochTime scheduleTime = epochTime(s_decade);
+        for (auto& pair : timers)
+            scheduleTime = std::min(pair.second, scheduleTime);
+        CFRunLoopTimerSetNextFireDate(timer.get(), scheduleTime.value());
+    }
+}
+#else
+JSRunLoopTimer::Manager::PerVMData::PerVMData(Manager& manager)
+    : runLoop(&RunLoop::current())
+    , timer(std::make_unique<RunLoop::Timer<Manager>>(*runLoop, &manager, &JSRunLoopTimer::Manager::timerDidFireCallback))
+{
+#if USE(GLIB_EVENT_LOOP)
+    timer->setPriority(RunLoopSourcePriority::JavascriptTimer);
+    timer->setName("[JavaScriptCore] JSRunLoopTimer");
+#endif
 }
 
-#if USE(CF)
+void JSRunLoopTimer::Manager::timerDidFireCallback()
+{
+    timerDidFire();
+}
+#endif
 
-JSRunLoopTimer::JSRunLoopTimer(VM* vm)
-    : m_vm(vm)
-    , m_apiLock(&vm->apiLock())
+JSRunLoopTimer::Manager::PerVMData::~PerVMData()
 {
-    m_vm->registerRunLoopTimer(this);
+#if USE(CF)
+    setRunLoop(nullptr, nullptr);
+#endif
 }
 
-void JSRunLoopTimer::setRunLoop(CFRunLoopRef runLoop)
+void JSRunLoopTimer::Manager::timerDidFire()
 {
-    if (m_runLoop) {
-        CFRunLoopRemoveTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
-        CFRunLoopTimerInvalidate(m_timer.get());
-        m_runLoop.clear();
-        m_timer.clear();
-    }
+    Vector<Ref<JSRunLoopTimer>> timersToFire;
 
-    m_runLoop = runLoop;
-    if (runLoop) {
-        memset(&m_context, 0, sizeof(CFRunLoopTimerContext));
-        m_context.info = this;
-        m_timer = adoptCF(CFRunLoopTimerCreate(kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + s_decade.seconds(), s_decade.seconds(), 0, 0, JSRunLoopTimer::timerDidFireCallback, &m_context));
-        CFRunLoopAddTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
+    {
+        auto locker = holdLock(m_lock);
+#if USE(CF)
+        CFRunLoopRef currentRunLoop = CFRunLoopGetCurrent();
+#else
+        RunLoop* currentRunLoop = &RunLoop::current();
+#endif
+        EpochTime nowEpochTime = epochTime(0_s);
+        for (auto& entry : m_mapping) {
+            PerVMData& data = entry.value;
+#if USE(CF)
+            if (data.runLoop.get() != currentRunLoop)
+                continue;
+#else
+            if (data.runLoop != currentRunLoop)
+                continue;
+#endif
+            
+            EpochTime scheduleTime = epochTime(s_decade);
+            for (size_t i = 0; i < data.timers.size(); ++i) {
+                {
+                    auto& pair = data.timers[i];
+                    if (pair.second > nowEpochTime) {
+                        scheduleTime = std::min(pair.second, scheduleTime);
+                        continue;
+                    }
+                    auto& last = data.timers.last();
+                    if (&last != &pair)
+                        std::swap(pair, last);
+                    --i;
+                }
+
+                auto pair = data.timers.takeLast();
+                timersToFire.append(WTFMove(pair.first));
+            }
+
+#if USE(CF)
+            CFRunLoopTimerSetNextFireDate(data.timer.get(), scheduleTime.value());
+#else
+            data.timer->startOneShot(std::max(0_s, scheduleTime - MonotonicTime::now().secondsSinceEpoch()));
+#endif
+        }
     }
+
+    for (auto& timer : timersToFire)
+        timer->timerDidFire();
 }
 
-JSRunLoopTimer::~JSRunLoopTimer()
+JSRunLoopTimer::Manager& JSRunLoopTimer::Manager::shared()
 {
-    JSLock* apiLock = m_apiLock.get();
-    std::lock_guard<JSLock> lock(*apiLock);
-    m_vm->unregisterRunLoopTimer(this);
-    m_apiLock = nullptr;
+    static Manager* manager;
+    static std::once_flag once;
+    std::call_once(once, [&] {
+        manager = new Manager;
+    });
+    return *manager;
 }
 
-void JSRunLoopTimer::timerDidFireCallback(CFRunLoopTimerRef, void* contextPtr)
+void JSRunLoopTimer::Manager::registerVM(VM& vm)
 {
-    static_cast<JSRunLoopTimer*>(contextPtr)->timerDidFire();
+    PerVMData data { *this };
+#if USE(CF)
+    data.setRunLoop(this, vm.runLoop());
+#endif
+
+    auto locker = holdLock(m_lock);
+    auto addResult = m_mapping.add({ vm.apiLock() }, WTFMove(data));
+    RELEASE_ASSERT(addResult.isNewEntry);
 }
 
-void JSRunLoopTimer::scheduleTimer(Seconds intervalInSeconds)
+void JSRunLoopTimer::Manager::unregisterVM(VM& vm)
 {
-    CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + intervalInSeconds.seconds());
-    m_isScheduled = true;
-    auto locker = holdLock(m_timerCallbacksLock);
-    for (auto& task : m_timerSetCallbacks)
-        task->run();
+    auto locker = holdLock(m_lock);
+
+    auto iter = m_mapping.find({ vm.apiLock() });
+    RELEASE_ASSERT(iter != m_mapping.end());
+    m_mapping.remove(iter);
 }
 
-void JSRunLoopTimer::cancelTimer()
+void JSRunLoopTimer::Manager::scheduleTimer(JSRunLoopTimer& timer, Seconds delay)
 {
-    CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + s_decade.seconds());
-    m_isScheduled = false;
+    EpochTime fireEpochTime = epochTime(delay);
+
+    auto locker = holdLock(m_lock);
+    auto iter = m_mapping.find(timer.m_apiLock);
+    RELEASE_ASSERT(iter != m_mapping.end()); // We don't allow calling this after the VM dies.
+
+    PerVMData& data = iter->value;
+    EpochTime scheduleTime = fireEpochTime;
+    bool found = false;
+    for (auto& entry : data.timers) {
+        if (entry.first.ptr() == &timer) {
+            entry.second = fireEpochTime;
+            found = true;
+        }
+        scheduleTime = std::min(scheduleTime, entry.second);
+    }
+
+    if (!found)
+        data.timers.append({ timer, fireEpochTime });
+
+#if USE(CF)
+    CFRunLoopTimerSetNextFireDate(data.timer.get(), scheduleTime.value());
+#else
+    data.timer->startOneShot(std::max(0_s, scheduleTime - MonotonicTime::now().secondsSinceEpoch()));
+#endif
 }
 
+void JSRunLoopTimer::Manager::cancelTimer(JSRunLoopTimer& timer)
+{
+    auto locker = holdLock(m_lock);
+    auto iter = m_mapping.find(timer.m_apiLock);
+    if (iter == m_mapping.end()) {
+        // It's trivial to allow this to be called after the VM dies, so we allow for it.
+        return;
+    }
+
+    PerVMData& data = iter->value;
+    EpochTime scheduleTime = epochTime(s_decade);
+    for (unsigned i = 0; i < data.timers.size(); ++i) {
+        {
+            auto& entry = data.timers[i];
+            if (entry.first.ptr() == &timer) {
+                RELEASE_ASSERT(timer.refCount() >= 2); // If we remove it from the entry below, we should not be the last thing pointing to it!
+                auto& last = data.timers.last();
+                if (&last != &entry)
+                    std::swap(entry, last);
+                data.timers.removeLast();
+                i--;
+                continue;
+            }
+        }
+
+        scheduleTime = std::min(scheduleTime, data.timers[i].second);
+    }
+
+#if USE(CF)
+    CFRunLoopTimerSetNextFireDate(data.timer.get(), scheduleTime.value());
 #else
+    data.timer->startOneShot(std::max(0_s, scheduleTime - MonotonicTime::now().secondsSinceEpoch()));
+#endif
+}
 
-JSRunLoopTimer::JSRunLoopTimer(VM* vm)
-    : m_vm(vm)
-    , m_apiLock(&vm->apiLock())
-    , m_timer(RunLoop::current(), this, &JSRunLoopTimer::timerDidFireCallback)
+std::optional<Seconds> JSRunLoopTimer::Manager::timeUntilFire(JSRunLoopTimer& timer)
 {
-#if USE(GLIB_EVENT_LOOP)
-    m_timer.setPriority(RunLoopSourcePriority::JavascriptTimer);
-    m_timer.setName("[JavaScriptCore] JSRunLoopTimer");
+    auto locker = holdLock(m_lock);
+    auto iter = m_mapping.find(timer.m_apiLock);
+    RELEASE_ASSERT(iter != m_mapping.end()); // We only allow this to be called with a live VM.
+
+    PerVMData& data = iter->value;
+    for (auto& entry : data.timers) {
+        if (entry.first.ptr() == &timer) {
+            EpochTime nowEpochTime = epochTime(0_s);
+            return entry.second - nowEpochTime;
+        }
+    }
+
+    return std::nullopt;
+}
+
+#if USE(CF)
+void JSRunLoopTimer::Manager::didChangeRunLoop(VM& vm, CFRunLoopRef newRunLoop)
+{
+    auto locker = holdLock(m_lock);
+    auto iter = m_mapping.find({ vm.apiLock() });
+    RELEASE_ASSERT(iter != m_mapping.end());
+
+    PerVMData& data = iter->value;
+    data.setRunLoop(this, newRunLoop);
+}
 #endif
-    m_timer.startOneShot(s_decade);
+
+void JSRunLoopTimer::timerDidFire()
+{
+    NO_TAIL_CALLS();
+
+    {
+        auto locker = holdLock(m_lock);
+        if (!m_isScheduled) {
+            // We raced between this callback being called and cancel() being called.
+            // That's fine, we just don't do anything here.
+            return;
+        }
+    }
+
+    std::lock_guard<JSLock> lock(m_apiLock.get());
+    RefPtr<VM> vm = m_apiLock->vm();
+    if (!vm) {
+        // The VM has been destroyed, so we should just give up.
+        return;
+    }
+
+    doWork(*vm);
+}
+
+JSRunLoopTimer::JSRunLoopTimer(VM* vm)
+    : m_apiLock(vm->apiLock())
+{
 }
 
 JSRunLoopTimer::~JSRunLoopTimer()
 {
 }
 
-void JSRunLoopTimer::timerDidFireCallback()
+std::optional<Seconds> JSRunLoopTimer::timeUntilFire()
 {
-    m_timer.startOneShot(s_decade);
-    timerDidFire();
+    return Manager::shared().timeUntilFire(*this);
 }
 
-void JSRunLoopTimer::scheduleTimer(Seconds intervalInSeconds)
+void JSRunLoopTimer::setTimeUntilFire(Seconds intervalInSeconds)
 {
-    m_timer.startOneShot(intervalInSeconds);
-    m_isScheduled = true;
+    {
+        auto locker = holdLock(m_lock);
+        m_isScheduled = true;
+        Manager::shared().scheduleTimer(*this, intervalInSeconds);
+    }
 
     auto locker = holdLock(m_timerCallbacksLock);
     for (auto& task : m_timerSetCallbacks)
@@ -155,12 +336,11 @@ void JSRunLoopTimer::scheduleTimer(Seconds intervalInSeconds)
 
 void JSRunLoopTimer::cancelTimer()
 {
-    m_timer.startOneShot(s_decade);
+    auto locker = holdLock(m_lock);
     m_isScheduled = false;
+    Manager::shared().cancelTimer(*this);
 }
 
-#endif
-
 void JSRunLoopTimer::addTimerSetNotification(TimerNotificationCallback callback)
 {
     auto locker = holdLock(m_timerCallbacksLock);
index 06c5f17..6651672 100644 (file)
@@ -47,54 +47,91 @@ public:
     typedef void TimerNotificationType();
     using TimerNotificationCallback = RefPtr<WTF::SharedTask<TimerNotificationType>>;
 
-    JSRunLoopTimer(VM*);
+    class Manager {
+#if USE(CF)
+        static void timerDidFireCallback(CFRunLoopTimerRef, void*);
+#else
+        void timerDidFireCallback();
+#endif
+
+        void timerDidFire();
+
+    public:
+        using EpochTime = Seconds;
+
+        static Manager& shared();
+        void registerVM(VM&);
+        void unregisterVM(VM&);
+        void scheduleTimer(JSRunLoopTimer&, Seconds nextFireTime);
+        void cancelTimer(JSRunLoopTimer&);
+
+        std::optional<Seconds> timeUntilFire(JSRunLoopTimer&);
+
+#if USE(CF)
+        void didChangeRunLoop(VM&, CFRunLoopRef newRunLoop);
+#endif
+
+    private:
+        Lock m_lock;
+
+        struct PerVMData {
+            PerVMData() = default;
+#if USE(CF)
+            PerVMData(Manager&) { }
+#else
+            PerVMData(Manager&);
+#endif
+            PerVMData(PerVMData&&) = default;
+            PerVMData& operator=(PerVMData&&) = default;
+
+            ~PerVMData();
+
 #if USE(CF)
-    static void timerDidFireCallback(CFRunLoopTimerRef, void*);
+            void setRunLoop(Manager*, CFRunLoopRef);
+            RetainPtr<CFRunLoopTimerRef> timer;
+            RetainPtr<CFRunLoopRef> runLoop;
+            CFRunLoopTimerContext context;
 #else
-    void timerDidFireCallback();
+            RunLoop* runLoop;
+            std::unique_ptr<RunLoop::Timer<Manager>> timer;
 #endif
+            Vector<std::pair<Ref<JSRunLoopTimer>, EpochTime>> timers;
+        };
 
+        HashMap<Ref<JSLock>, PerVMData> m_mapping;
+    };
+
+    JSRunLoopTimer(VM*);
     JS_EXPORT_PRIVATE virtual ~JSRunLoopTimer();
-    virtual void doWork() = 0;
+    virtual void doWork(VM&) = 0;
 
-    void scheduleTimer(Seconds intervalInSeconds);
+    void setTimeUntilFire(Seconds intervalInSeconds);
     void cancelTimer();
     bool isScheduled() const { return m_isScheduled; }
 
     // Note: The only thing the timer notification callback cannot do is
-    // call scheduleTimer(). This will cause a deadlock. It would not
+    // call setTimeUntilFire(). This will cause a deadlock. It would not
     // be hard to make this work, however, there are no clients that need
     // this behavior. We should implement it only if we find that we need it.
     JS_EXPORT_PRIVATE void addTimerSetNotification(TimerNotificationCallback);
     JS_EXPORT_PRIVATE void removeTimerSetNotification(TimerNotificationCallback);
 
-#if USE(CF)
-    JS_EXPORT_PRIVATE void setRunLoop(CFRunLoopRef);
-#endif // USE(CF)
+    JS_EXPORT_PRIVATE std::optional<Seconds> timeUntilFire();
 
 protected:
-    VM* m_vm;
-
     static const Seconds s_decade;
+    Ref<JSLock> m_apiLock;
 
-    RefPtr<JSLock> m_apiLock;
-    bool m_isScheduled { false };
-#if USE(CF)
-    RetainPtr<CFRunLoopTimerRef> m_timer;
-    RetainPtr<CFRunLoopRef> m_runLoop;
-
-    CFRunLoopTimerContext m_context;
+private:
+    friend class Manager;
 
-    Lock m_shutdownMutex;
-#else
-    RunLoop::Timer<JSRunLoopTimer> m_timer;
-#endif
+    void timerDidFire();
 
-    Lock m_timerCallbacksLock;
     HashSet<TimerNotificationCallback> m_timerSetCallbacks;
-    
-private:
-    void timerDidFire();
+    Lock m_timerCallbacksLock;
+
+    Lock m_lock;
+    bool m_isScheduled { false };
 };
     
 } // namespace JSC
index db851dc..343ee2a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -43,9 +43,9 @@ PromiseDeferredTimer::PromiseDeferredTimer(VM& vm)
 {
 }
 
-void PromiseDeferredTimer::doWork()
+void PromiseDeferredTimer::doWork(VM& vm)
 {
-    ASSERT(m_vm->currentThreadIsHoldingAPILock());
+    ASSERT(vm.currentThreadIsHoldingAPILock());
     m_taskLock.lock();
     cancelTimer();
     if (!m_runTasks) {
@@ -66,7 +66,7 @@ void PromiseDeferredTimer::doWork()
             m_taskLock.unlock(); 
 
             task();
-            m_vm->drainMicrotasks();
+            vm.drainMicrotasks();
 
             m_taskLock.lock();
             m_currentlyRunningTask = false;
@@ -75,7 +75,7 @@ void PromiseDeferredTimer::doWork()
 
     if (m_pendingPromises.isEmpty() && m_shouldStopRunLoopWhenAllPromisesFinish) {
 #if USE(CF)
-        CFRunLoopStop(m_runLoop.get());
+        CFRunLoopStop(vm.runLoop());
 #else
         RunLoop::current().stop();
 #endif
@@ -86,29 +86,30 @@ void PromiseDeferredTimer::doWork()
 
 void PromiseDeferredTimer::runRunLoop()
 {
-    ASSERT(!m_vm->currentThreadIsHoldingAPILock());
+    ASSERT(!m_apiLock->vm()->currentThreadIsHoldingAPILock());
 #if USE(CF)
-    ASSERT(CFRunLoopGetCurrent() == m_runLoop.get());
+    ASSERT(CFRunLoopGetCurrent() == m_apiLock->vm()->runLoop());
 #endif
     m_shouldStopRunLoopWhenAllPromisesFinish = true;
-    if (m_pendingPromises.size())
+    if (m_pendingPromises.size()) {
 #if USE(CF)
         CFRunLoopRun();
 #else
         RunLoop::run();
 #endif
+    }
 }
 
-void PromiseDeferredTimer::addPendingPromise(JSPromiseDeferred* ticket, Vector<Strong<JSCell>>&& dependencies)
+void PromiseDeferredTimer::addPendingPromise(VM& vm, JSPromiseDeferred* ticket, Vector<Strong<JSCell>>&& dependencies)
 {
-    ASSERT(m_vm->currentThreadIsHoldingAPILock());
+    ASSERT(vm.currentThreadIsHoldingAPILock());
     for (unsigned i = 0; i < dependencies.size(); ++i)
         ASSERT(dependencies[i].get() != ticket);
 
     auto result = m_pendingPromises.add(ticket, Vector<Strong<JSCell>>());
     if (result.isNewEntry) {
         dataLogLnIf(PromiseDeferredTimerInternal::verbose, "Adding new pending promise: ", RawPointer(ticket));
-        dependencies.append(Strong<JSCell>(*m_vm, ticket));
+        dependencies.append(Strong<JSCell>(vm, ticket));
         result.iterator->value = WTFMove(dependencies);
     } else {
         dataLogLnIf(PromiseDeferredTimerInternal::verbose, "Adding new dependencies for promise: ", RawPointer(ticket));
@@ -122,13 +123,13 @@ void PromiseDeferredTimer::addPendingPromise(JSPromiseDeferred* ticket, Vector<S
 
 bool PromiseDeferredTimer::hasPendingPromise(JSPromiseDeferred* ticket)
 {
-    ASSERT(m_vm->currentThreadIsHoldingAPILock());
+    ASSERT(ticket->vm()->currentThreadIsHoldingAPILock());
     return m_pendingPromises.contains(ticket);
 }
 
 bool PromiseDeferredTimer::hasDependancyInPendingPromise(JSPromiseDeferred* ticket, JSCell* dependency)
 {
-    ASSERT(m_vm->currentThreadIsHoldingAPILock());
+    ASSERT(ticket->vm()->currentThreadIsHoldingAPILock());
     ASSERT(m_pendingPromises.contains(ticket));
 
     auto result = m_pendingPromises.get(ticket);
@@ -137,7 +138,7 @@ bool PromiseDeferredTimer::hasDependancyInPendingPromise(JSPromiseDeferred* tick
 
 bool PromiseDeferredTimer::cancelPendingPromise(JSPromiseDeferred* ticket)
 {
-    ASSERT(m_vm->currentThreadIsHoldingAPILock());
+    ASSERT(ticket->vm()->currentThreadIsHoldingAPILock());
     bool result = m_pendingPromises.remove(ticket);
 
     if (result)
@@ -151,7 +152,7 @@ void PromiseDeferredTimer::scheduleWorkSoon(JSPromiseDeferred* ticket, Task&& ta
     LockHolder locker(m_taskLock);
     m_tasks.append(std::make_tuple(ticket, WTFMove(task)));
     if (!isScheduled() && !m_currentlyRunningTask)
-        scheduleTimer(0_s);
+        setTimeUntilFire(0_s);
 }
 
 } // namespace JSC
index 615b087..a52eb64 100644 (file)
@@ -44,9 +44,9 @@ public:
 
     PromiseDeferredTimer(VM&);
 
-    void doWork() override;
+    void doWork(VM&) override;
 
-    void addPendingPromise(JSPromiseDeferred*, Vector<Strong<JSCell>>&& dependencies);
+    void addPendingPromise(VM&, JSPromiseDeferred*, Vector<Strong<JSCell>>&& dependencies);
     JS_EXPORT_PRIVATE bool hasPendingPromise(JSPromiseDeferred* ticket);
     JS_EXPORT_PRIVATE bool hasDependancyInPendingPromise(JSPromiseDeferred* ticket, JSCell* dependency);
     // JSPromiseDeferred should handle canceling when the promise is resolved or rejected.
index 6f00520..f8e7e7f 100644 (file)
@@ -381,6 +381,8 @@ VM::VM(VMType vmType, HeapType heapType)
     updateSoftReservedZoneSize(Options::softReservedZoneSize());
     setLastStackTop(stack.origin());
 
+    JSRunLoopTimer::Manager::shared().registerVM(*this);
+
     // Need to be careful to keep everything consistent here
     JSLockHolder lock(this);
     AtomicStringTable* existingEntryAtomicStringTable = Thread::current().setCurrentAtomicStringTable(m_atomicStringTable);
@@ -581,6 +583,8 @@ VM::~VM()
     ASSERT(currentThreadIsHoldingAPILock());
     m_apiLock->willDestroyVM(this);
     heap.lastChanceToFinalize();
+
+    JSRunLoopTimer::Manager::shared().unregisterVM(*this);
     
     delete interpreter;
 #ifndef NDEBUG
@@ -1209,27 +1213,11 @@ void VM::verifyExceptionCheckNeedIsSatisfied(unsigned recursionDepth, ExceptionE
 #endif
 
 #if USE(CF)
-void VM::registerRunLoopTimer(JSRunLoopTimer* timer)
-{
-    ASSERT(runLoop());
-    ASSERT(!m_runLoopTimers.contains(timer));
-    m_runLoopTimers.add(timer);
-    timer->setRunLoop(runLoop());
-}
-
-void VM::unregisterRunLoopTimer(JSRunLoopTimer* timer)
-{
-    ASSERT(m_runLoopTimers.contains(timer));
-    m_runLoopTimers.remove(timer);
-    timer->setRunLoop(nullptr);
-}
-
 void VM::setRunLoop(CFRunLoopRef runLoop)
 {
     ASSERT(runLoop);
     m_runLoop = runLoop;
-    for (auto timer : m_runLoopTimers)
-        timer->setRunLoop(runLoop);
+    JSRunLoopTimer::Manager::shared().didChangeRunLoop(*this, runLoop);
 }
 #endif // USE(CF)
 
index c5bc9dd..a301aef 100644 (file)
@@ -307,7 +307,6 @@ private:
     RefPtr<JSLock> m_apiLock;
 #if USE(CF)
     // These need to be initialized before heap below.
-    HashSet<JSRunLoopTimer*> m_runLoopTimers;
     RetainPtr<CFRunLoopRef> m_runLoop;
 #endif
 
@@ -887,8 +886,6 @@ public:
 
 #if USE(CF)
     CFRunLoopRef runLoop() const { return m_runLoop.get(); }
-    void registerRunLoopTimer(JSRunLoopTimer*);
-    void unregisterRunLoopTimer(JSRunLoopTimer*);
     JS_EXPORT_PRIVATE void setRunLoop(CFRunLoopRef);
 #endif // USE(CF)
 
index 0866aaf..96d4c14 100644 (file)
@@ -91,7 +91,7 @@ static void webAssemblyModuleValidateAsyncInternal(ExecState* exec, JSPromiseDef
     Vector<Strong<JSCell>> dependencies;
     dependencies.append(Strong<JSCell>(vm, globalObject));
 
-    vm.promiseDeferredTimer->addPendingPromise(promise, WTFMove(dependencies));
+    vm.promiseDeferredTimer->addPendingPromise(vm, promise, WTFMove(dependencies));
 
     Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([promise, globalObject, &vm] (Wasm::Module::ValidationResult&& result) mutable {
         vm.promiseDeferredTimer->scheduleWorkSoon(promise, [promise, globalObject, result = WTFMove(result), &vm] () mutable {
@@ -173,7 +173,7 @@ static void instantiate(VM& vm, ExecState* exec, JSPromiseDeferred* promise, JSW
     // The instance keeps the module alive.
     dependencies.append(Strong<JSCell>(vm, instance));
     dependencies.append(Strong<JSCell>(vm, importObject));
-    vm.promiseDeferredTimer->addPendingPromise(promise, WTFMove(dependencies));
+    vm.promiseDeferredTimer->addPendingPromise(vm, promise, WTFMove(dependencies));
     // Note: This completion task may or may not get called immediately.
     module->module().compileAsync(&vm.wasmContext, instance->memoryMode(), createSharedTask<Wasm::CodeBlock::CallbackType>([promise, instance, module, importObject, resolveKind, creationMode, &vm] (Ref<Wasm::CodeBlock>&& refCodeBlock) mutable {
         RefPtr<Wasm::CodeBlock> codeBlock = WTFMove(refCodeBlock);
@@ -194,7 +194,7 @@ static void compileAndInstantiate(VM& vm, ExecState* exec, JSPromiseDeferred* pr
     Vector<Strong<JSCell>> dependencies;
     dependencies.append(Strong<JSCell>(vm, importObject));
     dependencies.append(Strong<JSCell>(vm, moduleKeyCell));
-    vm.promiseDeferredTimer->addPendingPromise(promise, WTFMove(dependencies));
+    vm.promiseDeferredTimer->addPendingPromise(vm, promise, WTFMove(dependencies));
 
     Vector<uint8_t> source = createSourceBufferFromValue(vm, exec, buffer);
     RETURN_IF_EXCEPTION(scope, reject(exec, scope, promise));
@@ -231,7 +231,7 @@ static void webAssemblyModuleInstantinateAsyncInternal(ExecState* exec, JSPromis
     Vector<Strong<JSCell>> dependencies;
     dependencies.append(Strong<JSCell>(vm, importObject));
     dependencies.append(Strong<JSCell>(vm, globalObject));
-    vm.promiseDeferredTimer->addPendingPromise(promise, WTFMove(dependencies));
+    vm.promiseDeferredTimer->addPendingPromise(vm, promise, WTFMove(dependencies));
 
     Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([promise, importObject, globalObject, &vm] (Wasm::Module::ValidationResult&& result) mutable {
         vm.promiseDeferredTimer->scheduleWorkSoon(promise, [promise, importObject, globalObject, result = WTFMove(result), &vm] () mutable {
@@ -310,7 +310,7 @@ EncodedJSValue JSC_HOST_CALL webAssemblyCompileStreamingInternal(ExecState* exec
 
     Vector<Strong<JSCell>> dependencies;
     dependencies.append(Strong<JSCell>(vm, globalObject));
-    vm.promiseDeferredTimer->addPendingPromise(promise, WTFMove(dependencies));
+    vm.promiseDeferredTimer->addPendingPromise(vm, promise, WTFMove(dependencies));
 
     if (globalObject->globalObjectMethodTable()->compileStreaming)
         globalObject->globalObjectMethodTable()->compileStreaming(globalObject, exec, promise, exec->argument(0));
@@ -347,7 +347,7 @@ EncodedJSValue JSC_HOST_CALL webAssemblyInstantiateStreamingInternal(ExecState*
                 Vector<Strong<JSCell>> dependencies;
                 dependencies.append(Strong<JSCell>(vm, globalObject));
                 dependencies.append(Strong<JSCell>(vm, importObject));
-                vm.promiseDeferredTimer->addPendingPromise(promise, WTFMove(dependencies));
+                vm.promiseDeferredTimer->addPendingPromise(vm, promise, WTFMove(dependencies));
 
                 // FIXME: <http://webkit.org/b/184888> if there's an importObject and it contains a Memory, then we can compile the module with the right memory type (fast or not) by looking at the memory's type.
                 globalObject->globalObjectMethodTable()->instantiateStreaming(globalObject, exec, promise, exec->argument(0), importObject);
index 57b14a1..8fea9d9 100644 (file)
@@ -1,3 +1,15 @@
+2018-08-23  Saam barati  <sbarati@apple.com>
+
+        JSRunLoopTimer may run part of a member function after it's destroyed
+        https://bugs.webkit.org/show_bug.cgi?id=188426
+
+        Reviewed by Mark Lam.
+
+        * page/cocoa/ResourceUsageThreadCocoa.mm:
+        (WebCore::ResourceUsageThread::platformThreadBody):
+        * page/linux/ResourceUsageThreadLinux.cpp:
+        (WebCore::ResourceUsageThread::platformThreadBody):
+
 2018-08-23  David Fenton  <david_fenton@apple.com>
 
         Unreviewed, rolling out r235129.
index 7e48224..fe5af8d 100644 (file)
@@ -248,8 +248,9 @@ void ResourceUsageThread::platformThreadBody(JSC::VM* vm, ResourceUsageData& dat
 
     data.totalExternalSize = currentGCOwnedExternal;
 
-    data.timeOfNextEdenCollection = vm->heap.edenActivityCallback()->nextFireTime();
-    data.timeOfNextFullCollection = vm->heap.fullActivityCallback()->nextFireTime();
+    auto now = MonotonicTime::now();
+    data.timeOfNextEdenCollection = now + vm->heap.edenActivityCallback()->timeUntilFire().value_or(Seconds(std::numeric_limits<double>::infinity()));
+    data.timeOfNextFullCollection = now + vm->heap.fullActivityCallback()->timeUntilFire().value_or(Seconds(std::numeric_limits<double>::infinity()));
 }
 
 }
index 70f0d69..dcafb6f 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2017 Igalia S.L.
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -168,8 +169,9 @@ void ResourceUsageThread::platformThreadBody(JSC::VM* vm, ResourceUsageData& dat
 
     data.totalExternalSize = currentGCOwnedExternal;
 
-    data.timeOfNextEdenCollection = vm->heap.edenActivityCallback()->nextFireTime();
-    data.timeOfNextFullCollection = vm->heap.fullActivityCallback()->nextFireTime();
+    auto now = MonotonicTime::now();
+    data.timeOfNextEdenCollection = now + vm->heap.edenActivityCallback()->timeUntilFire().value_or(Seconds(std::numeric_limits<double>::infinity()));
+    data.timeOfNextFullCollection = now + vm->heap.fullActivityCallback()->timeUntilFire().value_or(Seconds(std::numeric_limits<double>::infinity()));
 }
 
 } // namespace WebCore