Modified: trunk/Source/bmalloc/ChangeLog (203099 => 203100)
--- trunk/Source/bmalloc/ChangeLog 2016-07-11 23:53:42 UTC (rev 203099)
+++ trunk/Source/bmalloc/ChangeLog 2016-07-12 00:16:19 UTC (rev 203100)
@@ -1,3 +1,37 @@
+2016-07-11 Geoffrey Garen <[email protected]>
+
+ 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 <[email protected]>
Crash in com.apple.WebKit.WebContent at std::__1::__call_once_proxy<std::__1::tuple<CrashReporterSupportLibrary()::$_0&&> >
Modified: trunk/Source/bmalloc/bmalloc/AsyncTask.h (203099 => 203100)
--- trunk/Source/bmalloc/bmalloc/AsyncTask.h 2016-07-11 23:53:42 UTC (rev 203099)
+++ trunk/Source/bmalloc/bmalloc/AsyncTask.h 2016-07-12 00:16:19 UTC (rev 203100)
@@ -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: trunk/Source/bmalloc/bmalloc/Heap.h (203099 => 203100)
--- trunk/Source/bmalloc/bmalloc/Heap.h 2016-07-11 23:53:42 UTC (rev 203099)
+++ trunk/Source/bmalloc/bmalloc/Heap.h 2016-07-12 00:16:19 UTC (rev 203100)
@@ -26,6 +26,7 @@
#ifndef Heap_h
#define Heap_h
+#include "AsyncTask.h"
#include "BumpRange.h"
#include "Environment.h"
#include "LineMetadata.h"
Modified: trunk/Source/bmalloc/bmalloc/VMHeap.h (203099 => 203100)
--- trunk/Source/bmalloc/bmalloc/VMHeap.h 2016-07-11 23:53:42 UTC (rev 203099)
+++ trunk/Source/bmalloc/bmalloc/VMHeap.h 2016-07-12 00:16:19 UTC (rev 203100)
@@ -26,7 +26,6 @@
#ifndef VMHeap_h
#define VMHeap_h
-#include "AsyncTask.h"
#include "Chunk.h"
#include "FixedVector.h"
#include "Map.h"
