[iOS] Web process gets suspended while holding locked database files
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Apr 2019 18:06:09 +0000 (18:06 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Apr 2019 18:06:09 +0000 (18:06 +0000)
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:

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

Source/WebCore/ChangeLog
Source/WebCore/platform/sql/SQLiteDatabase.cpp
Source/WebCore/platform/sql/SQLiteDatabase.h
Source/WebCore/platform/sql/SQLiteDatabaseTracker.cpp
Source/WebKit/ChangeLog
Source/WebKit/Shared/WebSQLiteDatabaseTracker.cpp
Source/WebKit/Shared/WebSQLiteDatabaseTracker.h
Source/WebKit/WebProcess/WebProcess.cpp
Source/WebKit/WebProcess/WebProcess.h

index e40d8fa..5ba0aa4 100644 (file)
@@ -1,3 +1,23 @@
+2019-04-05  Sihui Liu  <sihui_liu@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-queue@webkit.org>
 
         Unreviewed, rolling out r243833.
index 1ec93b9..3ff57ed 100644 (file)
@@ -71,6 +71,15 @@ static void initializeSQLiteIfNecessary()
     });
 }
 
+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 @@ bool SQLiteDatabase::open(const String& filename, OpenMode openMode)
 
     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();
index 9a4c526..149866f 100644 (file)
@@ -138,6 +138,8 @@ public:
     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*);
 
index e2ae0ce..82c343d 100644 (file)
@@ -40,18 +40,16 @@ static Lock transactionInProgressMutex;
 
 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 incrementTransactionInProgressCount()
 
 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 @@ void decrementTransactionInProgressCount()
 #if !ASSERT_DISABLED
 bool hasTransactionInProgress()
 {
+    std::lock_guard<Lock> lock(transactionInProgressMutex);
     return !s_staticSQLiteDatabaseTrackerClient || s_transactionInProgressCounter > 0;
 }
 #endif
index 6252d0a..5d6f1ab 100644 (file)
@@ -1,3 +1,22 @@
+2019-04-05  Sihui Liu  <sihui_liu@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-queue@webkit.org>
 
         Unreviewed, rolling out r243833.
index 56657e5..cc69f5e 100644 (file)
@@ -52,6 +52,15 @@ WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker(NetworkProcess& process)
     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] {
index b382320..0f65232 100644 (file)
@@ -40,6 +40,8 @@ public:
     explicit WebSQLiteDatabaseTracker(WebProcess&);
     explicit WebSQLiteDatabaseTracker(NetworkProcess&);
 
+    ~WebSQLiteDatabaseTracker();
+
     // WebCore::SQLiteDatabaseTrackerClient
     void willBeginFirstTransaction() override;
     void didFinishLastTransaction() override;
index 343177d..2add5a7 100644 (file)
 #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 @@ WebProcess::WebProcess()
 #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 @@ void WebProcess::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend shou
 #endif
 
 #if PLATFORM(IOS_FAMILY)
+    m_webSQLiteDatabaseTracker = nullptr;
+    SQLiteDatabase::setIsDatabaseOpeningForbidden(true);
+    DatabaseTracker::singleton().closeAllDatabases(CurrentQueryBehavior::Interrupt);
     accessibilityProcessSuspendedNotification(true);
     updateFreezerStatus();
 #endif
@@ -1489,7 +1496,6 @@ void WebProcess::processWillSuspendImminently(CompletionHandler<void(bool)>&& co
     }
 
     RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently()", this);
-    DatabaseTracker::singleton().closeAllDatabases(CurrentQueryBehavior::Interrupt);
     actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::No);
     completionHandler(true);
 }
@@ -1506,6 +1512,8 @@ void WebProcess::cancelPrepareToSuspend()
     unfreezeAllLayerTrees();
 
 #if PLATFORM(IOS_FAMILY)
+    m_webSQLiteDatabaseTracker = std::make_unique<WebSQLiteDatabaseTracker>(*this);
+    SQLiteDatabase::setIsDatabaseOpeningForbidden(false);
     accessibilityProcessSuspendedNotification(false);
 #endif
 
@@ -1577,6 +1585,8 @@ void WebProcess::processDidResume()
     unfreezeAllLayerTrees();
     
 #if PLATFORM(IOS_FAMILY)
+    m_webSQLiteDatabaseTracker = std::make_unique<WebSQLiteDatabaseTracker>(*this);
+    SQLiteDatabase::setIsDatabaseOpeningForbidden(false);
     accessibilityProcessSuspendedNotification(false);
 #endif
 
index 1e42cd5..ee781a0 100644 (file)
 #include <wtf/MachSendRight.h>
 #endif
 
-#if PLATFORM(IOS_FAMILY)
-#include "WebSQLiteDatabaseTracker.h"
-#endif
-
 namespace API {
 class Object;
 }
@@ -112,6 +108,7 @@ enum class WebsiteDataType;
 struct WebPageCreationParameters;
 struct WebPageGroupData;
 struct WebPreferencesStore;
+class WebSQLiteDatabaseTracker;
 struct WebsiteData;
 struct WebsiteDataStoreParameters;
 
@@ -499,7 +496,7 @@ private:
     RefPtr<WebCore::ApplicationCacheStorage> m_applicationCacheStorage;
 
 #if PLATFORM(IOS_FAMILY)
-    WebSQLiteDatabaseTracker m_webSQLiteDatabaseTracker;
+    std::unique_ptr<WebSQLiteDatabaseTracker> m_webSQLiteDatabaseTracker;
 #endif
 
     enum PageMarkingLayersAsVolatileCounterType { };