Add a null check in VMTraps::willDestroyVM() to handle a race condition.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Mar 2017 19:29:26 +0000 (19:29 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Mar 2017 19:29:26 +0000 (19:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169620

Reviewed by Filip Pizlo.

There exists a race between VMTraps::willDestroyVM() (which removed SignalSenders
from its m_signalSenders list) and SignalSender::send() (which removes itself
from the list).  In the event that SignalSender::send() removes itself between
the time that VMTraps::willDestroyVM() checks if m_signalSenders is empty and the
time it takes a sender from m_signalSenders, VMTraps::willDestroyVM() may end up
with a NULL sender pointer.  The fix is to add the missing null check before using
the sender pointer.

* runtime/VMTraps.cpp:
(JSC::VMTraps::willDestroyVM):
(JSC::VMTraps::fireTrap):
* runtime/VMTraps.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/VMTraps.cpp
Source/JavaScriptCore/runtime/VMTraps.h

index 30dfc69..c644694 100644 (file)
@@ -1,5 +1,25 @@
 2017-03-14  Mark Lam  <mark.lam@apple.com>
 
+        Add a null check in VMTraps::willDestroyVM() to handle a race condition.
+        https://bugs.webkit.org/show_bug.cgi?id=169620
+
+        Reviewed by Filip Pizlo.
+
+        There exists a race between VMTraps::willDestroyVM() (which removed SignalSenders
+        from its m_signalSenders list) and SignalSender::send() (which removes itself
+        from the list).  In the event that SignalSender::send() removes itself between
+        the time that VMTraps::willDestroyVM() checks if m_signalSenders is empty and the
+        time it takes a sender from m_signalSenders, VMTraps::willDestroyVM() may end up
+        with a NULL sender pointer.  The fix is to add the missing null check before using
+        the sender pointer.
+
+        * runtime/VMTraps.cpp:
+        (JSC::VMTraps::willDestroyVM):
+        (JSC::VMTraps::fireTrap):
+        * runtime/VMTraps.h:
+
+2017-03-14  Mark Lam  <mark.lam@apple.com>
+
         Gardening: Speculative build fix for CLoop after r213886.
         https://bugs.webkit.org/show_bug.cgi?id=169436
 
index 16b05be..30e5a3e 100644 (file)
@@ -403,6 +403,8 @@ VMTraps::VMTraps()
 
 void VMTraps::willDestroyVM()
 {
+    m_isShuttingDown = true;
+    WTF::storeStoreFence();
 #if ENABLE(SIGNAL_BASED_VM_TRAPS)
     while (!m_signalSenders.isEmpty()) {
         RefPtr<SignalSender> sender;
@@ -413,9 +415,12 @@ void VMTraps::willDestroyVM()
             // to acquire these locks in the opposite order.
             auto locker = holdLock(m_lock);
             sender = m_signalSenders.takeAny();
+            if (!sender)
+                break;
         }
         sender->willDestroyVM();
     }
+    ASSERT(m_signalSenders.isEmpty());
 #endif
 }
 
@@ -476,6 +481,7 @@ void VMTraps::fireTrap(VMTraps::EventType eventType)
     ASSERT(!vm().currentThreadIsHoldingAPILock());
     {
         auto locker = holdLock(m_lock);
+        ASSERT(!m_isShuttingDown);
         setTrapForEvent(locker, eventType);
         m_needToInvalidatedCodeBlocks = true;
     }
index b046a01..fdba2af 100644 (file)
@@ -167,6 +167,7 @@ private:
         BitField m_trapsBitField;
     };
     bool m_needToInvalidatedCodeBlocks { false };
+    bool m_isShuttingDown { false };
 
 #if ENABLE(SIGNAL_BASED_VM_TRAPS)
     HashSet<RefPtr<SignalSender>> m_signalSenders;