Title: [204712] releases/WebKitGTK/webkit-2.12/Source/bmalloc
Revision
204712
Author
carlo...@webkit.org
Date
2016-08-22 06:51:47 -0700 (Mon, 22 Aug 2016)

Log Message

Merge r203100 - Crash due to abort() calling libc++.1.dylib: std::__1::thread::detach()
https://bugs.webkit.org/show_bug.cgi?id=159655

Reviewed by Sam Weinig.

It's not entirely clear what was happening in these crashes, but our
use of detach() was 100% forward-looking, so we can just remove it for
now.

This patch removes the ability for the scavenger owner to die before
the scavenger thread dies (which was unused) and also removes the
ability for the scavenger thread to exit (which was used, but we
messed up and did thread joining lazily, so we never got any benefit
from thread exit.)

We can add these features back when we need them, and make them work then.

* bmalloc/AsyncTask.h:
(bmalloc::Function>::AsyncTask): We start out in the running state now
because we know that starting our thread will run it.

(bmalloc::Function>::~AsyncTask): We don't support destruction anymore.

(bmalloc::Function>::runSlowCase): I removed the Exited state.

(bmalloc::Function>::threadRunLoop): I removed the Exited and
ExitRequested states.

* bmalloc/Heap.h:

* bmalloc/VMHeap.h:

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.12/Source/bmalloc/ChangeLog (204711 => 204712)


--- releases/WebKitGTK/webkit-2.12/Source/bmalloc/ChangeLog	2016-08-22 13:51:38 UTC (rev 204711)
+++ releases/WebKitGTK/webkit-2.12/Source/bmalloc/ChangeLog	2016-08-22 13:51:47 UTC (rev 204712)
@@ -1,3 +1,37 @@
+2016-07-11  Geoffrey Garen  <gga...@apple.com>
+
+        Crash due to abort() calling libc++.1.dylib: std::__1::thread::detach()
+        https://bugs.webkit.org/show_bug.cgi?id=159655
+
+        Reviewed by Sam Weinig.
+
+        It's not entirely clear what was happening in these crashes, but our
+        use of detach() was 100% forward-looking, so we can just remove it for
+        now.
+
+        This patch removes the ability for the scavenger owner to die before
+        the scavenger thread dies (which was unused) and also removes the
+        ability for the scavenger thread to exit (which was used, but we
+        messed up and did thread joining lazily, so we never got any benefit
+        from thread exit.)
+
+        We can add these features back when we need them, and make them work then.
+
+        * bmalloc/AsyncTask.h:
+        (bmalloc::Function>::AsyncTask): We start out in the running state now
+        because we know that starting our thread will run it.
+
+        (bmalloc::Function>::~AsyncTask): We don't support destruction anymore.
+
+        (bmalloc::Function>::runSlowCase): I removed the Exited state.
+
+        (bmalloc::Function>::threadRunLoop): I removed the Exited and
+        ExitRequested states.
+
+        * bmalloc/Heap.h:
+
+        * bmalloc/VMHeap.h:
+
 2016-06-12  David Kilzer  <ddkil...@apple.com>
 
         Crash in com.apple.WebKit.WebContent at std::__1::__call_once_proxy<std::__1::tuple<CrashReporterSupportLibrary()::$_0&&> >

Modified: releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/AsyncTask.h (204711 => 204712)


--- releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/AsyncTask.h	2016-08-22 13:51:38 UTC (rev 204711)
+++ releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/AsyncTask.h	2016-08-22 13:51:47 UTC (rev 204712)
@@ -44,10 +44,8 @@
     void run();
 
 private:
-    enum State { Exited, ExitRequested, Sleeping, Running, RunRequested };
+    enum State { Sleeping, Running, RunRequested };
 
-    static const constexpr std::chrono::seconds exitDelay = std::chrono::seconds(1);
-
     void runSlowCase();
 
     static void threadEntryPoint(AsyncTask*);
@@ -64,13 +62,11 @@
     Function m_function;
 };
 
-template<typename Object, typename Function> const constexpr std::chrono::seconds AsyncTask<Object, Function>::exitDelay;
-
 template<typename Object, typename Function>
 AsyncTask<Object, Function>::AsyncTask(Object& object, const Function& function)
-    : m_state(Exited)
+    : m_state(Running)
     , m_condition()
-    , m_thread()
+    , m_thread(std::thread(&AsyncTask::threadEntryPoint, this))
     , m_object(object)
     , m_function(function)
 {
@@ -79,19 +75,9 @@
 template<typename Object, typename Function>
 AsyncTask<Object, Function>::~AsyncTask()
 {
-    // Prevent our thread from entering the running or sleeping state.
-    State oldState = m_state.exchange(ExitRequested);
-
-    // Wake our thread if it was already in the sleeping state.
-    if (oldState == Sleeping) {
-        std::lock_guard<Mutex> lock(m_conditionMutex);
-        m_condition.notify_all();
-    }
-
-    // Wait for our thread to exit because it uses our data members (and it may
-    // use m_object's data members).
-    if (m_thread.joinable())
-        m_thread.join();
+    // We'd like to mark our destructor deleted but C++ won't allow it because
+    // we are an automatic member of Heap.
+    RELEASE_BASSERT(0);
 }
 
 template<typename Object, typename Function>
@@ -109,16 +95,9 @@
     if (oldState == RunRequested || oldState == Running)
         return;
 
-    if (oldState == Sleeping) {
-        std::lock_guard<Mutex> lock(m_conditionMutex);
-        m_condition.notify_all();
-        return;
-    }
-
-    BASSERT(oldState == Exited);
-    if (m_thread.joinable())
-        m_thread.detach();
-    m_thread = std::thread(&AsyncTask::threadEntryPoint, this);
+    BASSERT(oldState == Sleeping);
+    std::lock_guard<Mutex> lock(m_conditionMutex);
+    m_condition.notify_all();
 }
 
 template<typename Object, typename Function>
@@ -130,10 +109,9 @@
 template<typename Object, typename Function>
 void AsyncTask<Object, Function>::threadRunLoop()
 {
-    // This loop ratchets downward from most active to least active state, and
-    // finally exits. While we ratchet downward, any other thread may reset our
-    // state to RunRequested or ExitRequested.
-    
+    // This loop ratchets downward from most active to least active state. While
+    // we ratchet downward, any other thread may reset our state.
+
     // We require any state change while we are sleeping to signal to our
     // condition variable and wake us up.
 
@@ -145,16 +123,8 @@
         expectedState = Running;
         if (m_state.compare_exchange_weak(expectedState, Sleeping)) {
             std::unique_lock<Mutex> lock(m_conditionMutex);
-            m_condition.wait_for(lock, exitDelay, [=]() { return this->m_state != Sleeping; });
+            m_condition.wait(lock, [&]() { return m_state != Sleeping; });
         }
-
-        expectedState = Sleeping;
-        if (m_state.compare_exchange_weak(expectedState, Exited))
-            return;
-        
-        expectedState = ExitRequested;
-        if (m_state.compare_exchange_weak(expectedState, Exited))
-            return;
     }
 }
 

Modified: releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/Heap.h (204711 => 204712)


--- releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/Heap.h	2016-08-22 13:51:38 UTC (rev 204711)
+++ releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/Heap.h	2016-08-22 13:51:47 UTC (rev 204712)
@@ -26,6 +26,7 @@
 #ifndef Heap_h
 #define Heap_h
 
+#include "AsyncTask.h"
 #include "BumpRange.h"
 #include "Environment.h"
 #include "LineMetadata.h"

Modified: releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/VMHeap.h (204711 => 204712)


--- releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/VMHeap.h	2016-08-22 13:51:38 UTC (rev 204711)
+++ releases/WebKitGTK/webkit-2.12/Source/bmalloc/bmalloc/VMHeap.h	2016-08-22 13:51:47 UTC (rev 204712)
@@ -26,7 +26,6 @@
 #ifndef VMHeap_h
 #define VMHeap_h
 
-#include "AsyncTask.h"
 #include "Chunk.h"
 #include "FixedVector.h"
 #include "Map.h"
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to