Title: [243939] trunk/Source
Revision
243939
Author
sihui_...@apple.com
Date
2019-04-05 11:06:09 -0700 (Fri, 05 Apr 2019)

Log Message

[iOS] Web process gets suspended while holding locked database files
https://bugs.webkit.org/show_bug.cgi?id=196519
<rdar://problem/49531797>

Reviewed by Chris Dumez.

Source/WebCore:

We should close all databases and make sure not open new databases when web process is ready to suspend.

* platform/sql/SQLiteDatabase.cpp:
(WebCore::SQLiteDatabase::setIsDatabaseOpeningForbidden):
(WebCore::SQLiteDatabase::open):
* platform/sql/SQLiteDatabase.h:
* platform/sql/SQLiteDatabaseTracker.cpp:
(WebCore::SQLiteDatabaseTracker::setClient):
(WebCore::SQLiteDatabaseTracker::incrementTransactionInProgressCount):
(WebCore::SQLiteDatabaseTracker::decrementTransactionInProgressCount):
(WebCore::SQLiteDatabaseTracker::hasTransactionInProgress):

Source/WebKit:

* Shared/WebSQLiteDatabaseTracker.cpp:
(WebKit::WebSQLiteDatabaseTracker::~WebSQLiteDatabaseTracker):
* Shared/WebSQLiteDatabaseTracker.h:
* WebProcess/WebProcess.cpp:
(WebKit::m_webSQLiteDatabaseTracker):
(WebKit::WebProcess::actualPrepareToSuspend):
(WebKit::WebProcess::processWillSuspendImminently):
(WebKit::WebProcess::cancelPrepareToSuspend):
(WebKit::WebProcess::processDidResume):
* WebProcess/WebProcess.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (243938 => 243939)


--- trunk/Source/WebCore/ChangeLog	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebCore/ChangeLog	2019-04-05 18:06:09 UTC (rev 243939)
@@ -1,3 +1,23 @@
+2019-04-05  Sihui Liu  <sihui_...@apple.com>
+
+        [iOS] Web process gets suspended while holding locked database files
+        https://bugs.webkit.org/show_bug.cgi?id=196519
+        <rdar://problem/49531797>
+
+        Reviewed by Chris Dumez.
+
+        We should close all databases and make sure not open new databases when web process is ready to suspend.
+
+        * platform/sql/SQLiteDatabase.cpp:
+        (WebCore::SQLiteDatabase::setIsDatabaseOpeningForbidden):
+        (WebCore::SQLiteDatabase::open):
+        * platform/sql/SQLiteDatabase.h:
+        * platform/sql/SQLiteDatabaseTracker.cpp:
+        (WebCore::SQLiteDatabaseTracker::setClient):
+        (WebCore::SQLiteDatabaseTracker::incrementTransactionInProgressCount):
+        (WebCore::SQLiteDatabaseTracker::decrementTransactionInProgressCount):
+        (WebCore::SQLiteDatabaseTracker::hasTransactionInProgress):
+
 2019-04-05  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, rolling out r243833.

Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp (243938 => 243939)


--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp	2019-04-05 18:06:09 UTC (rev 243939)
@@ -71,6 +71,15 @@
     });
 }
 
+static bool isDatabaseOpeningForbidden = false;
+static Lock isDatabaseOpeningForbiddenMutex;
+
+void SQLiteDatabase::setIsDatabaseOpeningForbidden(bool isForbidden)
+{
+    std::lock_guard<Lock> lock(isDatabaseOpeningForbiddenMutex);
+    isDatabaseOpeningForbidden = isForbidden;
+}
+
 SQLiteDatabase::SQLiteDatabase() = default;
 
 SQLiteDatabase::~SQLiteDatabase()
@@ -84,27 +93,35 @@
 
     close();
 
-    int flags = SQLITE_OPEN_AUTOPROXY;
-    switch (openMode) {
-    case OpenMode::ReadOnly:
-        flags |= SQLITE_OPEN_READONLY;
-        break;
-    case OpenMode::ReadWrite:
-        flags |= SQLITE_OPEN_READWRITE;
-        break;
-    case OpenMode::ReadWriteCreate:
-        flags |= SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
-        break;
-    }
+    {
+        std::lock_guard<Lock> lock(isDatabaseOpeningForbiddenMutex);
+        if (isDatabaseOpeningForbidden) {
+            m_openErrorMessage = "opening database is forbidden";
+            return false;
+        }
 
-    m_openError = sqlite3_open_v2(FileSystem::fileSystemRepresentation(filename).data(), &m_db, flags, nullptr);
-    if (m_openError != SQLITE_OK) {
-        m_openErrorMessage = m_db ? sqlite3_errmsg(m_db) : "sqlite_open returned null";
-        LOG_ERROR("SQLite database failed to load from %s\nCause - %s", filename.ascii().data(),
-            m_openErrorMessage.data());
-        sqlite3_close(m_db);
-        m_db = 0;
-        return false;
+        int flags = SQLITE_OPEN_AUTOPROXY;
+        switch (openMode) {
+        case OpenMode::ReadOnly:
+            flags |= SQLITE_OPEN_READONLY;
+            break;
+        case OpenMode::ReadWrite:
+            flags |= SQLITE_OPEN_READWRITE;
+            break;
+        case OpenMode::ReadWriteCreate:
+            flags |= SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
+            break;
+        }
+
+        m_openError = sqlite3_open_v2(FileSystem::fileSystemRepresentation(filename).data(), &m_db, flags, nullptr);
+        if (m_openError != SQLITE_OK) {
+            m_openErrorMessage = m_db ? sqlite3_errmsg(m_db) : "sqlite_open returned null";
+            LOG_ERROR("SQLite database failed to load from %s\nCause - %s", filename.ascii().data(),
+                m_openErrorMessage.data());
+            sqlite3_close(m_db);
+            m_db = 0;
+            return false;
+        }
     }
 
     overrideUnauthorizedFunctions();

Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.h (243938 => 243939)


--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.h	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.h	2019-04-05 18:06:09 UTC (rev 243939)
@@ -138,6 +138,8 @@
     WEBCORE_EXPORT void disableThreadingChecks() {}
 #endif
 
+    WEBCORE_EXPORT static void setIsDatabaseOpeningForbidden(bool);
+
 private:
     static int authorizerFunction(void*, int, const char*, const char*, const char*, const char*);
 

Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabaseTracker.cpp (243938 => 243939)


--- trunk/Source/WebCore/platform/sql/SQLiteDatabaseTracker.cpp	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabaseTracker.cpp	2019-04-05 18:06:09 UTC (rev 243939)
@@ -40,18 +40,16 @@
 
 void setClient(SQLiteDatabaseTrackerClient* client)
 {
-    ASSERT(client);
-    ASSERT(!s_staticSQLiteDatabaseTrackerClient || s_staticSQLiteDatabaseTrackerClient == client);
+    std::lock_guard<Lock> lock(transactionInProgressMutex);
     s_staticSQLiteDatabaseTrackerClient = client;
 }
 
 void incrementTransactionInProgressCount()
 {
+    std::lock_guard<Lock> lock(transactionInProgressMutex);
     if (!s_staticSQLiteDatabaseTrackerClient)
         return;
 
-    std::lock_guard<Lock> lock(transactionInProgressMutex);
-
     s_transactionInProgressCounter++;
     if (s_transactionInProgressCounter == 1)
         s_staticSQLiteDatabaseTrackerClient->willBeginFirstTransaction();
@@ -59,11 +57,10 @@
 
 void decrementTransactionInProgressCount()
 {
+    std::lock_guard<Lock> lock(transactionInProgressMutex);
     if (!s_staticSQLiteDatabaseTrackerClient)
         return;
 
-    std::lock_guard<Lock> lock(transactionInProgressMutex);
-
     ASSERT(s_transactionInProgressCounter);
     s_transactionInProgressCounter--;
 
@@ -74,6 +71,7 @@
 #if !ASSERT_DISABLED
 bool hasTransactionInProgress()
 {
+    std::lock_guard<Lock> lock(transactionInProgressMutex);
     return !s_staticSQLiteDatabaseTrackerClient || s_transactionInProgressCounter > 0;
 }
 #endif

Modified: trunk/Source/WebKit/ChangeLog (243938 => 243939)


--- trunk/Source/WebKit/ChangeLog	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebKit/ChangeLog	2019-04-05 18:06:09 UTC (rev 243939)
@@ -1,3 +1,22 @@
+2019-04-05  Sihui Liu  <sihui_...@apple.com>
+
+        [iOS] Web process gets suspended while holding locked database files
+        https://bugs.webkit.org/show_bug.cgi?id=196519
+        <rdar://problem/49531797>
+
+        Reviewed by Chris Dumez.
+
+        * Shared/WebSQLiteDatabaseTracker.cpp:
+        (WebKit::WebSQLiteDatabaseTracker::~WebSQLiteDatabaseTracker):
+        * Shared/WebSQLiteDatabaseTracker.h:
+        * WebProcess/WebProcess.cpp:
+        (WebKit::m_webSQLiteDatabaseTracker):
+        (WebKit::WebProcess::actualPrepareToSuspend):
+        (WebKit::WebProcess::processWillSuspendImminently):
+        (WebKit::WebProcess::cancelPrepareToSuspend):
+        (WebKit::WebProcess::processDidResume):
+        * WebProcess/WebProcess.h:
+
 2019-04-05  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, rolling out r243833.

Modified: trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.cpp (243938 => 243939)


--- trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.cpp	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.cpp	2019-04-05 18:06:09 UTC (rev 243939)
@@ -52,6 +52,15 @@
     SQLiteDatabaseTracker::setClient(this);
 }
 
+WebSQLiteDatabaseTracker::~WebSQLiteDatabaseTracker()
+{
+    ASSERT(RunLoop::isMain());
+    SQLiteDatabaseTracker::setClient(nullptr);
+
+    if (m_hysteresis.state() == PAL::HysteresisState::Started)
+        hysteresisUpdated(PAL::HysteresisState::Stopped);
+}
+
 void WebSQLiteDatabaseTracker::willBeginFirstTransaction()
 {
     callOnMainThread([this] {

Modified: trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.h (243938 => 243939)


--- trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.h	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.h	2019-04-05 18:06:09 UTC (rev 243939)
@@ -40,6 +40,8 @@
     explicit WebSQLiteDatabaseTracker(WebProcess&);
     explicit WebSQLiteDatabaseTracker(NetworkProcess&);
 
+    ~WebSQLiteDatabaseTracker();
+
     // WebCore::SQLiteDatabaseTrackerClient
     void willBeginFirstTransaction() override;
     void didFinishLastTransaction() override;

Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (243938 => 243939)


--- trunk/Source/WebKit/WebProcess/WebProcess.cpp	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp	2019-04-05 18:06:09 UTC (rev 243939)
@@ -137,6 +137,10 @@
 #include "UserMediaCaptureManager.h"
 #endif
 
+#if PLATFORM(IOS_FAMILY)
+#include "WebSQLiteDatabaseTracker.h"
+#endif
+
 #if ENABLE(SEC_ITEM_SHIM)
 #include "SecItemShim.h"
 #endif
@@ -184,7 +188,7 @@
 #endif
     , m_nonVisibleProcessCleanupTimer(*this, &WebProcess::nonVisibleProcessCleanupTimerFired)
 #if PLATFORM(IOS_FAMILY)
-    , m_webSQLiteDatabaseTracker(*this)
+    , m_webSQLiteDatabaseTracker(std::make_unique<WebSQLiteDatabaseTracker>(*this))
 #endif
 {
     // Initialize our platform strategies.
@@ -1461,6 +1465,9 @@
 #endif
 
 #if PLATFORM(IOS_FAMILY)
+    m_webSQLiteDatabaseTracker = nullptr;
+    SQLiteDatabase::setIsDatabaseOpeningForbidden(true);
+    DatabaseTracker::singleton().closeAllDatabases(CurrentQueryBehavior::Interrupt);
     accessibilityProcessSuspendedNotification(true);
     updateFreezerStatus();
 #endif
@@ -1489,7 +1496,6 @@
     }
 
     RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently()", this);
-    DatabaseTracker::singleton().closeAllDatabases(CurrentQueryBehavior::Interrupt);
     actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::No);
     completionHandler(true);
 }
@@ -1506,6 +1512,8 @@
     unfreezeAllLayerTrees();
 
 #if PLATFORM(IOS_FAMILY)
+    m_webSQLiteDatabaseTracker = std::make_unique<WebSQLiteDatabaseTracker>(*this);
+    SQLiteDatabase::setIsDatabaseOpeningForbidden(false);
     accessibilityProcessSuspendedNotification(false);
 #endif
 
@@ -1577,6 +1585,8 @@
     unfreezeAllLayerTrees();
     
 #if PLATFORM(IOS_FAMILY)
+    m_webSQLiteDatabaseTracker = std::make_unique<WebSQLiteDatabaseTracker>(*this);
+    SQLiteDatabase::setIsDatabaseOpeningForbidden(false);
     accessibilityProcessSuspendedNotification(false);
 #endif
 

Modified: trunk/Source/WebKit/WebProcess/WebProcess.h (243938 => 243939)


--- trunk/Source/WebKit/WebProcess/WebProcess.h	2019-04-05 18:01:22 UTC (rev 243938)
+++ trunk/Source/WebKit/WebProcess/WebProcess.h	2019-04-05 18:06:09 UTC (rev 243939)
@@ -57,10 +57,6 @@
 #include <wtf/MachSendRight.h>
 #endif
 
-#if PLATFORM(IOS_FAMILY)
-#include "WebSQLiteDatabaseTracker.h"
-#endif
-
 namespace API {
 class Object;
 }
@@ -112,6 +108,7 @@
 struct WebPageCreationParameters;
 struct WebPageGroupData;
 struct WebPreferencesStore;
+class WebSQLiteDatabaseTracker;
 struct WebsiteData;
 struct WebsiteDataStoreParameters;
 
@@ -499,7 +496,7 @@
     RefPtr<WebCore::ApplicationCacheStorage> m_applicationCacheStorage;
 
 #if PLATFORM(IOS_FAMILY)
-    WebSQLiteDatabaseTracker m_webSQLiteDatabaseTracker;
+    std::unique_ptr<WebSQLiteDatabaseTracker> m_webSQLiteDatabaseTracker;
 #endif
 
     enum PageMarkingLayersAsVolatileCounterType { };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to