<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[203100] trunk/Source/bmalloc</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/203100">203100</a></dd>
<dt>Author</dt> <dd>ggaren@apple.com</dd>
<dt>Date</dt> <dd>2016-07-11 17:16:19 -0700 (Mon, 11 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourcebmallocChangeLog">trunk/Source/bmalloc/ChangeLog</a></li>
<li><a href="#trunkSourcebmallocbmallocAsyncTaskh">trunk/Source/bmalloc/bmalloc/AsyncTask.h</a></li>
<li><a href="#trunkSourcebmallocbmallocHeaph">trunk/Source/bmalloc/bmalloc/Heap.h</a></li>
<li><a href="#trunkSourcebmallocbmallocVMHeaph">trunk/Source/bmalloc/bmalloc/VMHeap.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourcebmallocChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/bmalloc/ChangeLog (203099 => 203100)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2016-07-11 Geoffrey Garen <ggaren@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:
+
</ins><span class="cx"> 2016-06-12 David Kilzer <ddkilzer@apple.com>
</span><span class="cx">
</span><span class="cx"> Crash in com.apple.WebKit.WebContent at std::__1::__call_once_proxy<std::__1::tuple<CrashReporterSupportLibrary()::$_0&&> >
</span></span></pre></div>
<a id="trunkSourcebmallocbmallocAsyncTaskh"></a>
<div class="modfile"><h4>Modified: trunk/Source/bmalloc/bmalloc/AsyncTask.h (203099 => 203100)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -44,10 +44,8 @@
</span><span class="cx"> void run();
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- enum State { Exited, ExitRequested, Sleeping, Running, RunRequested };
</del><ins>+ enum State { Sleeping, Running, RunRequested };
</ins><span class="cx">
</span><del>- static const constexpr std::chrono::seconds exitDelay = std::chrono::seconds(1);
-
</del><span class="cx"> void runSlowCase();
</span><span class="cx">
</span><span class="cx"> static void threadEntryPoint(AsyncTask*);
</span><span class="lines">@@ -64,13 +62,11 @@
</span><span class="cx"> Function m_function;
</span><span class="cx"> };
</span><span class="cx">
</span><del>-template<typename Object, typename Function> const constexpr std::chrono::seconds AsyncTask<Object, Function>::exitDelay;
-
</del><span class="cx"> template<typename Object, typename Function>
</span><span class="cx"> AsyncTask<Object, Function>::AsyncTask(Object& object, const Function& function)
</span><del>- : m_state(Exited)
</del><ins>+ : m_state(Running)
</ins><span class="cx"> , m_condition()
</span><del>- , m_thread()
</del><ins>+ , m_thread(std::thread(&AsyncTask::threadEntryPoint, this))
</ins><span class="cx"> , m_object(object)
</span><span class="cx"> , m_function(function)
</span><span class="cx"> {
</span><span class="lines">@@ -79,19 +75,9 @@
</span><span class="cx"> template<typename Object, typename Function>
</span><span class="cx"> AsyncTask<Object, Function>::~AsyncTask()
</span><span class="cx"> {
</span><del>- // 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();
</del><ins>+ // 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);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template<typename Object, typename Function>
</span><span class="lines">@@ -109,16 +95,9 @@
</span><span class="cx"> if (oldState == RunRequested || oldState == Running)
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- 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);
</del><ins>+ BASSERT(oldState == Sleeping);
+ std::lock_guard<Mutex> lock(m_conditionMutex);
+ m_condition.notify_all();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template<typename Object, typename Function>
</span><span class="lines">@@ -130,10 +109,9 @@
</span><span class="cx"> template<typename Object, typename Function>
</span><span class="cx"> void AsyncTask<Object, Function>::threadRunLoop()
</span><span class="cx"> {
</span><del>- // 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.
-
</del><ins>+ // This loop ratchets downward from most active to least active state. While
+ // we ratchet downward, any other thread may reset our state.
+
</ins><span class="cx"> // We require any state change while we are sleeping to signal to our
</span><span class="cx"> // condition variable and wake us up.
</span><span class="cx">
</span><span class="lines">@@ -145,16 +123,8 @@
</span><span class="cx"> expectedState = Running;
</span><span class="cx"> if (m_state.compare_exchange_weak(expectedState, Sleeping)) {
</span><span class="cx"> std::unique_lock<Mutex> lock(m_conditionMutex);
</span><del>- m_condition.wait_for(lock, exitDelay, [=]() { return this->m_state != Sleeping; });
</del><ins>+ m_condition.wait(lock, [&]() { return m_state != Sleeping; });
</ins><span class="cx"> }
</span><del>-
- expectedState = Sleeping;
- if (m_state.compare_exchange_weak(expectedState, Exited))
- return;
-
- expectedState = ExitRequested;
- if (m_state.compare_exchange_weak(expectedState, Exited))
- return;
</del><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourcebmallocbmallocHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/bmalloc/bmalloc/Heap.h (203099 => 203100)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -26,6 +26,7 @@
</span><span class="cx"> #ifndef Heap_h
</span><span class="cx"> #define Heap_h
</span><span class="cx">
</span><ins>+#include "AsyncTask.h"
</ins><span class="cx"> #include "BumpRange.h"
</span><span class="cx"> #include "Environment.h"
</span><span class="cx"> #include "LineMetadata.h"
</span></span></pre></div>
<a id="trunkSourcebmallocbmallocVMHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/bmalloc/bmalloc/VMHeap.h (203099 => 203100)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -26,7 +26,6 @@
</span><span class="cx"> #ifndef VMHeap_h
</span><span class="cx"> #define VMHeap_h
</span><span class="cx">
</span><del>-#include "AsyncTask.h"
</del><span class="cx"> #include "Chunk.h"
</span><span class="cx"> #include "FixedVector.h"
</span><span class="cx"> #include "Map.h"
</span></span></pre>
</div>
</div>
</body>
</html>