<!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>[161995] trunk/Source/WebCore</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/161995">161995</a></dd>
<dt>Author</dt> <dd>mrowe@apple.com</dd>
<dt>Date</dt> <dd>2014-01-14 12:28:23 -0800 (Tue, 14 Jan 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>WebCore icon database appears to leak sudden termination assertions
&lt;https://webkit.org/b/126971&gt; / &lt;rdar://problem/15808797&gt;

Introduce an RAII wrapper around disableSuddenTermination / enableSuddenTermination
and adopt it in IconDatabase to address the incorrect management of sudden termination.

IconDatabase now owns up to two SuddenTerminationDisabler objects. One ensures that
sudden termination is disabled while we're waiting on the sync timer to fire. The second
ensures that sudden termination is disabled while we're waiting on the sync thread to
process any pending work.

Reviewed by Alexey Proskuryakov.

* loader/icon/IconDatabase.cpp:
(WebCore::IconDatabase::IconDatabase):
(WebCore::IconDatabase::wakeSyncThread): Disable sudden termination until the sync thread
has finished this unit of work.
(WebCore::IconDatabase::scheduleOrDeferSyncTimer): Disable sudden termination until the
sync timer has fired.
(WebCore::IconDatabase::syncTimerFired): Clear the member variable to reenable sudden termination.
(WebCore::IconDatabase::syncThreadMainLoop): Taken ownership of the SuddenTerminationDisabler
instance when we start processing a unit of work. Discard the object when our work is complete.
* loader/icon/IconDatabase.h:
* platform/SuddenTermination.h:
(WebCore::SuddenTerminationDisabler::SuddenTerminationDisabler): Disable sudden termination when created.
(WebCore::SuddenTerminationDisabler::~SuddenTerminationDisabler): Enable it when destroyed.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreloadericonIconDatabasecpp">trunk/Source/WebCore/loader/icon/IconDatabase.cpp</a></li>
<li><a href="#trunkSourceWebCoreloadericonIconDatabaseh">trunk/Source/WebCore/loader/icon/IconDatabase.h</a></li>
<li><a href="#trunkSourceWebCoreplatformSuddenTerminationh">trunk/Source/WebCore/platform/SuddenTermination.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (161994 => 161995)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2014-01-14 20:23:41 UTC (rev 161994)
+++ trunk/Source/WebCore/ChangeLog        2014-01-14 20:28:23 UTC (rev 161995)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2014-01-14  Mark Rowe  &lt;mrowe@apple.com&gt;
+
+        WebCore icon database appears to leak sudden termination assertions
+        &lt;https://webkit.org/b/126971&gt; / &lt;rdar://problem/15808797&gt;
+
+        Introduce an RAII wrapper around disableSuddenTermination / enableSuddenTermination
+        and adopt it in IconDatabase to address the incorrect management of sudden termination.
+
+        IconDatabase now owns up to two SuddenTerminationDisabler objects. One ensures that
+        sudden termination is disabled while we're waiting on the sync timer to fire. The second
+        ensures that sudden termination is disabled while we're waiting on the sync thread to
+        process any pending work.
+
+        Reviewed by Alexey Proskuryakov.
+
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::IconDatabase):
+        (WebCore::IconDatabase::wakeSyncThread): Disable sudden termination until the sync thread
+        has finished this unit of work.
+        (WebCore::IconDatabase::scheduleOrDeferSyncTimer): Disable sudden termination until the
+        sync timer has fired.
+        (WebCore::IconDatabase::syncTimerFired): Clear the member variable to reenable sudden termination.
+        (WebCore::IconDatabase::syncThreadMainLoop): Taken ownership of the SuddenTerminationDisabler
+        instance when we start processing a unit of work. Discard the object when our work is complete.
+        * loader/icon/IconDatabase.h:
+        * platform/SuddenTermination.h:
+        (WebCore::SuddenTerminationDisabler::SuddenTerminationDisabler): Disable sudden termination when created.
+        (WebCore::SuddenTerminationDisabler::~SuddenTerminationDisabler): Enable it when destroyed.
+
</ins><span class="cx"> 2014-01-14  Joseph Pecoraro  &lt;pecoraro@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: For Remote Inspection link WebProcess's to their parent UIProcess
</span></span></pre></div>
<a id="trunkSourceWebCoreloadericonIconDatabasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/icon/IconDatabase.cpp (161994 => 161995)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/icon/IconDatabase.cpp        2014-01-14 20:23:41 UTC (rev 161994)
+++ trunk/Source/WebCore/loader/icon/IconDatabase.cpp        2014-01-14 20:28:23 UTC (rev 161995)
</span><span class="lines">@@ -797,7 +797,6 @@
</span><span class="cx">     , m_removeIconsRequested(false)
</span><span class="cx">     , m_iconURLImportComplete(false)
</span><span class="cx">     , m_syncThreadHasWorkToDo(false)
</span><del>-    , m_disabledSuddenTerminationForSyncThread(false)
</del><span class="cx">     , m_retainOrReleaseIconRequested(false)
</span><span class="cx">     , m_initialPruningComplete(false)
</span><span class="cx">     , m_client(defaultClient())
</span><span class="lines">@@ -838,14 +837,8 @@
</span><span class="cx"> {
</span><span class="cx">     MutexLocker locker(m_syncLock);
</span><span class="cx"> 
</span><del>-    if (!m_disabledSuddenTerminationForSyncThread) {
-        m_disabledSuddenTerminationForSyncThread = true;
-        // The following is balanced by the call to enableSuddenTermination in the
-        // syncThreadMainLoop function.
-        // FIXME: It would be better to only disable sudden termination if we have
-        // something to write, not just if we have something to read.
-        disableSuddenTermination();
-    }
</del><ins>+    if (!m_disableSuddenTerminationWhileSyncThreadHasWorkToDo)
+        m_disableSuddenTerminationWhileSyncThreadHasWorkToDo = std::make_unique&lt;SuddenTerminationDisabler&gt;();
</ins><span class="cx"> 
</span><span class="cx">     m_syncThreadHasWorkToDo = true;
</span><span class="cx">     m_syncCondition.signal();
</span><span class="lines">@@ -869,9 +862,8 @@
</span><span class="cx">     if (m_scheduleOrDeferSyncTimerRequested)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    // The following is balanced by the call to enableSuddenTermination in the
-    // syncTimerFired function.
-    disableSuddenTermination();
</del><ins>+    if (!m_disableSuddenTerminationWhileSyncTimerScheduled)
+        m_disableSuddenTerminationWhileSyncTimerScheduled = std::make_unique&lt;SuddenTerminationDisabler&gt;();
</ins><span class="cx"> 
</span><span class="cx">     m_scheduleOrDeferSyncTimerRequested = true;
</span><span class="cx">     callOnMainThread(performScheduleOrDeferSyncTimerOnMainThread, this);
</span><span class="lines">@@ -882,9 +874,7 @@
</span><span class="cx">     ASSERT_NOT_SYNC_THREAD();
</span><span class="cx">     wakeSyncThread();
</span><span class="cx"> 
</span><del>-    // The following is balanced by the call to disableSuddenTermination in the
-    // scheduleOrDeferSyncTimer function.
-    enableSuddenTermination();
</del><ins>+    m_disableSuddenTerminationWhileSyncTimerScheduled.reset();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> // ******************
</span><span class="lines">@@ -1363,8 +1353,7 @@
</span><span class="cx"> 
</span><span class="cx">     m_syncLock.lock();
</span><span class="cx"> 
</span><del>-    bool shouldReenableSuddenTermination = m_disabledSuddenTerminationForSyncThread;
-    m_disabledSuddenTerminationForSyncThread = false;
</del><ins>+    std::unique_ptr&lt;SuddenTerminationDisabler&gt; disableSuddenTermination = std::move(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo);
</ins><span class="cx"> 
</span><span class="cx">     // We'll either do any pending work on our first pass through the loop, or we'll terminate
</span><span class="cx">     // without doing any work. Either way we're dealing with any currently-pending work.
</span><span class="lines">@@ -1444,37 +1433,21 @@
</span><span class="cx">         if (shouldStopThreadActivity())
</span><span class="cx">             continue;
</span><span class="cx"> 
</span><del>-        if (shouldReenableSuddenTermination) {
-            // The following is balanced by the call to disableSuddenTermination in the
-            // wakeSyncThread function. Any time we wait on the condition, we also have
-            // to enableSuddenTermation, after doing the next batch of work.
-            enableSuddenTermination();
-        }
</del><ins>+        disableSuddenTermination.reset();
</ins><span class="cx"> 
</span><span class="cx">         while (!m_syncThreadHasWorkToDo)
</span><span class="cx">             m_syncCondition.wait(m_syncLock);
</span><span class="cx"> 
</span><span class="cx">         m_syncThreadHasWorkToDo = false;
</span><span class="cx"> 
</span><del>-        ASSERT(m_disabledSuddenTerminationForSyncThread);
-        shouldReenableSuddenTermination = true;
-        m_disabledSuddenTerminationForSyncThread = false;
</del><ins>+        ASSERT(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo);
+        disableSuddenTermination = std::move(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo);
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     m_syncLock.unlock();
</span><span class="cx">     
</span><span class="cx">     // Thread is terminating at this point
</span><span class="cx">     cleanupSyncThread();
</span><del>-
-    if (shouldReenableSuddenTermination) {
-        // The following is balanced by the call to disableSuddenTermination in the
-        // wakeSyncThread function. Any time we wait on the condition, we also have
-        // to enableSuddenTermation, after doing the next batch of work.
-        enableSuddenTermination();
-
-        MutexLocker locker(m_syncLock);
-        m_disabledSuddenTerminationForSyncThread = false;
-    }
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void IconDatabase::performPendingRetainAndReleaseOperations()
</span></span></pre></div>
<a id="trunkSourceWebCoreloadericonIconDatabaseh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/icon/IconDatabase.h (161994 => 161995)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/icon/IconDatabase.h        2014-01-14 20:23:41 UTC (rev 161994)
+++ trunk/Source/WebCore/loader/icon/IconDatabase.h        2014-01-14 20:28:23 UTC (rev 161995)
</span><span class="lines">@@ -55,6 +55,7 @@
</span><span class="cx"> class PageURLRecord;
</span><span class="cx"> class PageURLSnapshot;
</span><span class="cx"> class SharedBuffer;
</span><ins>+class SuddenTerminationDisabler;
</ins><span class="cx"> 
</span><span class="cx"> #if ENABLE(ICONDATABASE)
</span><span class="cx"> class SQLTransaction;
</span><span class="lines">@@ -141,6 +142,7 @@
</span><span class="cx">     void performScheduleOrDeferSyncTimer();
</span><span class="cx"> 
</span><span class="cx">     bool m_scheduleOrDeferSyncTimerRequested;
</span><ins>+    std::unique_ptr&lt;SuddenTerminationDisabler&gt; m_disableSuddenTerminationWhileSyncTimerScheduled;
</ins><span class="cx"> 
</span><span class="cx"> // *** Any Thread ***
</span><span class="cx"> public:
</span><span class="lines">@@ -165,7 +167,7 @@
</span><span class="cx">     bool m_removeIconsRequested;
</span><span class="cx">     bool m_iconURLImportComplete;
</span><span class="cx">     bool m_syncThreadHasWorkToDo;
</span><del>-    bool m_disabledSuddenTerminationForSyncThread;
</del><ins>+    std::unique_ptr&lt;SuddenTerminationDisabler&gt; m_disableSuddenTerminationWhileSyncThreadHasWorkToDo;
</ins><span class="cx"> 
</span><span class="cx">     Mutex m_urlAndIconLock;
</span><span class="cx">     // Holding m_urlAndIconLock is required when accessing any of the following data structures or the objects they contain
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformSuddenTerminationh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/SuddenTermination.h (161994 => 161995)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/SuddenTermination.h        2014-01-14 20:23:41 UTC (rev 161994)
+++ trunk/Source/WebCore/platform/SuddenTermination.h        2014-01-14 20:28:23 UTC (rev 161995)
</span><span class="lines">@@ -26,6 +26,8 @@
</span><span class="cx"> #ifndef SuddenTermination_h
</span><span class="cx"> #define SuddenTermination_h
</span><span class="cx"> 
</span><ins>+#include &lt;wtf/Noncopyable.h&gt;
+
</ins><span class="cx"> namespace WebCore {
</span><span class="cx"> 
</span><span class="cx">     // Once disabled via one or more more calls to disableSuddenTermination(), fast shutdown
</span><span class="lines">@@ -39,6 +41,20 @@
</span><span class="cx">     inline void enableSuddenTermination() { }
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><ins>+    class SuddenTerminationDisabler {
+        WTF_MAKE_NONCOPYABLE(SuddenTerminationDisabler);
+    public:
+        SuddenTerminationDisabler()
+        {
+            disableSuddenTermination();
+        }
+
+        ~SuddenTerminationDisabler()
+        {
+            enableSuddenTermination();
+        }
+    };
+
</ins><span class="cx"> } // namespace WebCore
</span><span class="cx"> 
</span><span class="cx"> #endif // SuddenTermination_h
</span></span></pre>
</div>
</div>

</body>
</html>