Modified: trunk/Source/bmalloc/ChangeLog (191740 => 191741)
--- trunk/Source/bmalloc/ChangeLog 2015-10-29 18:35:17 UTC (rev 191740)
+++ trunk/Source/bmalloc/ChangeLog 2015-10-29 18:45:11 UTC (rev 191741)
@@ -1,3 +1,41 @@
+2015-10-29 Geoffrey Garen <gga...@apple.com>
+
+ bmalloc: AsyncTask should handle destruction
+ https://bugs.webkit.org/show_bug.cgi?id=150648
+
+ Reviewed by Mark Lam.
+
+ So we can use it in more places.
+
+ * bmalloc/AsyncTask.h: Use std::thread instead of pthread because it
+ should be more portable.
+
+ (bmalloc::Function>::AsyncTask): Renamed Signaled to RunRequested for
+ clarity. Added an ExitRequested state.
+
+ (bmalloc::Function>::~AsyncTask): Wait for our child thread to exit
+ before destroying ourselves because our child thread will modify our
+ data (and might modify our client's data). Note that we only need to
+ wait for the last child thread since any prior child thread, having
+ reached the Exited condition, is guaranteed not to read or write any
+ data.
+
+ (bmalloc::Function>::run):
+ (bmalloc::Function>::runSlowCase): Updated for interface changes. Also
+ changed to use our WebKit style for condition signal: Hold the lock
+ during the signal and always notify all. Technically, neither is necessary,
+ but it is easier to understand the code this way, and harder to make
+ mistakes.
+
+ (bmalloc::Function>::threadEntryPoint):
+ (bmalloc::Function>::threadRunLoop): Handle the new ExitRequested state.
+ Technically, this state has no meaningful difference from the Exited
+ state, but it is nice to be explicit.
+
+ (bmalloc::Function>::join): Deleted.
+ (bmalloc::Function>::pthreadEntryPoint): Deleted.
+ (bmalloc::Function>::entryPoint): Deleted.
+
2015-10-15 Geoffrey Garen <gga...@apple.com>
bmalloc: per-thread cache data structure should be smaller
Modified: trunk/Source/bmalloc/bmalloc/AsyncTask.h (191740 => 191741)
--- trunk/Source/bmalloc/bmalloc/AsyncTask.h 2015-10-29 18:35:17 UTC (rev 191740)
+++ trunk/Source/bmalloc/bmalloc/AsyncTask.h 2015-10-29 18:45:11 UTC (rev 191741)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -31,7 +31,6 @@
#include "Mutex.h"
#include <atomic>
#include <condition_variable>
-#include <pthread.h>
#include <thread>
namespace bmalloc {
@@ -40,26 +39,27 @@
class AsyncTask {
public:
AsyncTask(Object&, const Function&);
+ ~AsyncTask();
void run();
- void join();
private:
- enum State { Exited, Sleeping, Running, Signaled };
+ enum State { Exited, ExitRequested, Sleeping, Running, RunRequested };
static const constexpr std::chrono::seconds exitDelay = std::chrono::seconds(1);
void runSlowCase();
- static void* pthreadEntryPoint(void*);
- void entryPoint();
+ static void threadEntryPoint(AsyncTask*);
+ void threadRunLoop();
std::atomic<State> m_state;
Mutex m_conditionMutex;
std::condition_variable_any m_condition;
- pthread_t m_thread;
+ std::thread m_thread;
+
Object& m_object;
Function m_function;
};
@@ -77,22 +77,27 @@
}
template<typename Object, typename Function>
-void AsyncTask<Object, Function>::join()
+AsyncTask<Object, Function>::~AsyncTask()
{
- if (m_state == Exited)
- return;
+ // Prevent our thread from entering the running or sleeping state.
+ State oldState = m_state.exchange(ExitRequested);
- { std::lock_guard<Mutex> lock(m_conditionMutex); }
- m_condition.notify_one();
+ // 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();
+ }
- while (m_state != Exited)
- std::this_thread::yield();
+ // 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();
}
template<typename Object, typename Function>
inline void AsyncTask<Object, Function>::run()
{
- if (m_state == Signaled)
+ if (m_state == RunRequested)
return;
runSlowCase();
}
@@ -100,33 +105,40 @@
template<typename Object, typename Function>
NO_INLINE void AsyncTask<Object, Function>::runSlowCase()
{
- State oldState = m_state.exchange(Signaled);
- if (oldState == Signaled || oldState == Running)
+ State oldState = m_state.exchange(RunRequested);
+ if (oldState == RunRequested || oldState == Running)
return;
if (oldState == Sleeping) {
- { std::lock_guard<Mutex> lock(m_conditionMutex); }
- m_condition.notify_one();
+ std::lock_guard<Mutex> lock(m_conditionMutex);
+ m_condition.notify_all();
return;
}
BASSERT(oldState == Exited);
- pthread_create(&m_thread, nullptr, &pthreadEntryPoint, this);
- pthread_detach(m_thread);
+ if (m_thread.joinable())
+ m_thread.detach();
+ m_thread = std::thread(&AsyncTask::threadEntryPoint, this);
}
template<typename Object, typename Function>
-void* AsyncTask<Object, Function>::pthreadEntryPoint(void* asyncTask)
+void AsyncTask<Object, Function>::threadEntryPoint(AsyncTask* asyncTask)
{
- static_cast<AsyncTask*>(asyncTask)->entryPoint();
- return nullptr;
+ asyncTask->threadRunLoop();
}
template<typename Object, typename Function>
-void AsyncTask<Object, Function>::entryPoint()
+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.
+
+ // We require any state change while we are sleeping to signal to our
+ // condition variable and wake us up.
+
while (1) {
- State expectedState = Signaled;
+ State expectedState = RunRequested;
if (m_state.compare_exchange_weak(expectedState, Running))
(m_object.*m_function)();
@@ -139,6 +151,10 @@
expectedState = Sleeping;
if (m_state.compare_exchange_weak(expectedState, Exited))
return;
+
+ expectedState = ExitRequested;
+ if (m_state.compare_exchange_weak(expectedState, Exited))
+ return;
}
}
