WebCore icon database appears to leak sudden termination assertions
authormrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jan 2014 20:28:23 +0000 (20:28 +0000)
committermrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jan 2014 20:28:23 +0000 (20:28 +0000)
<https://webkit.org/b/126971> / <rdar://problem/15808797>

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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@161995 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/icon/IconDatabase.cpp
Source/WebCore/loader/icon/IconDatabase.h
Source/WebCore/platform/SuddenTermination.h

index 132c94e..eb2d655 100644 (file)
@@ -1,3 +1,32 @@
+2014-01-14  Mark Rowe  <mrowe@apple.com>
+
+        WebCore icon database appears to leak sudden termination assertions
+        <https://webkit.org/b/126971> / <rdar://problem/15808797>
+
+        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.
+
 2014-01-14  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: For Remote Inspection link WebProcess's to their parent UIProcess
index 5222288..f0cbd64 100644 (file)
@@ -797,7 +797,6 @@ IconDatabase::IconDatabase()
     , m_removeIconsRequested(false)
     , m_iconURLImportComplete(false)
     , m_syncThreadHasWorkToDo(false)
-    , m_disabledSuddenTerminationForSyncThread(false)
     , m_retainOrReleaseIconRequested(false)
     , m_initialPruningComplete(false)
     , m_client(defaultClient())
@@ -838,14 +837,8 @@ void IconDatabase::wakeSyncThread()
 {
     MutexLocker locker(m_syncLock);
 
-    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();
-    }
+    if (!m_disableSuddenTerminationWhileSyncThreadHasWorkToDo)
+        m_disableSuddenTerminationWhileSyncThreadHasWorkToDo = std::make_unique<SuddenTerminationDisabler>();
 
     m_syncThreadHasWorkToDo = true;
     m_syncCondition.signal();
@@ -869,9 +862,8 @@ void IconDatabase::scheduleOrDeferSyncTimer()
     if (m_scheduleOrDeferSyncTimerRequested)
         return;
 
-    // The following is balanced by the call to enableSuddenTermination in the
-    // syncTimerFired function.
-    disableSuddenTermination();
+    if (!m_disableSuddenTerminationWhileSyncTimerScheduled)
+        m_disableSuddenTerminationWhileSyncTimerScheduled = std::make_unique<SuddenTerminationDisabler>();
 
     m_scheduleOrDeferSyncTimerRequested = true;
     callOnMainThread(performScheduleOrDeferSyncTimerOnMainThread, this);
@@ -882,9 +874,7 @@ void IconDatabase::syncTimerFired(Timer<IconDatabase>&)
     ASSERT_NOT_SYNC_THREAD();
     wakeSyncThread();
 
-    // The following is balanced by the call to disableSuddenTermination in the
-    // scheduleOrDeferSyncTimer function.
-    enableSuddenTermination();
+    m_disableSuddenTerminationWhileSyncTimerScheduled.reset();
 }
 
 // ******************
@@ -1363,8 +1353,7 @@ void IconDatabase::syncThreadMainLoop()
 
     m_syncLock.lock();
 
-    bool shouldReenableSuddenTermination = m_disabledSuddenTerminationForSyncThread;
-    m_disabledSuddenTerminationForSyncThread = false;
+    std::unique_ptr<SuddenTerminationDisabler> disableSuddenTermination = std::move(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo);
 
     // We'll either do any pending work on our first pass through the loop, or we'll terminate
     // without doing any work. Either way we're dealing with any currently-pending work.
@@ -1444,37 +1433,21 @@ void IconDatabase::syncThreadMainLoop()
         if (shouldStopThreadActivity())
             continue;
 
-        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();
-        }
+        disableSuddenTermination.reset();
 
         while (!m_syncThreadHasWorkToDo)
             m_syncCondition.wait(m_syncLock);
 
         m_syncThreadHasWorkToDo = false;
 
-        ASSERT(m_disabledSuddenTerminationForSyncThread);
-        shouldReenableSuddenTermination = true;
-        m_disabledSuddenTerminationForSyncThread = false;
+        ASSERT(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo);
+        disableSuddenTermination = std::move(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo);
     }
 
     m_syncLock.unlock();
     
     // Thread is terminating at this point
     cleanupSyncThread();
-
-    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;
-    }
 }
 
 void IconDatabase::performPendingRetainAndReleaseOperations()
index 1c38bd0..04db323 100644 (file)
@@ -55,6 +55,7 @@ class URL;
 class PageURLRecord;
 class PageURLSnapshot;
 class SharedBuffer;
+class SuddenTerminationDisabler;
 
 #if ENABLE(ICONDATABASE)
 class SQLTransaction;
@@ -141,6 +142,7 @@ private:
     void performScheduleOrDeferSyncTimer();
 
     bool m_scheduleOrDeferSyncTimerRequested;
+    std::unique_ptr<SuddenTerminationDisabler> m_disableSuddenTerminationWhileSyncTimerScheduled;
 
 // *** Any Thread ***
 public:
@@ -165,7 +167,7 @@ private:
     bool m_removeIconsRequested;
     bool m_iconURLImportComplete;
     bool m_syncThreadHasWorkToDo;
-    bool m_disabledSuddenTerminationForSyncThread;
+    std::unique_ptr<SuddenTerminationDisabler> m_disableSuddenTerminationWhileSyncThreadHasWorkToDo;
 
     Mutex m_urlAndIconLock;
     // Holding m_urlAndIconLock is required when accessing any of the following data structures or the objects they contain
index 80967c6..49558f6 100644 (file)
@@ -26,6 +26,8 @@
 #ifndef SuddenTermination_h
 #define SuddenTermination_h
 
+#include <wtf/Noncopyable.h>
+
 namespace WebCore {
 
     // Once disabled via one or more more calls to disableSuddenTermination(), fast shutdown
@@ -39,6 +41,20 @@ namespace WebCore {
     inline void enableSuddenTermination() { }
 #endif
 
+    class SuddenTerminationDisabler {
+        WTF_MAKE_NONCOPYABLE(SuddenTerminationDisabler);
+    public:
+        SuddenTerminationDisabler()
+        {
+            disableSuddenTermination();
+        }
+
+        ~SuddenTerminationDisabler()
+        {
+            enableSuddenTermination();
+        }
+    };
+
 } // namespace WebCore
 
 #endif // SuddenTermination_h