Modified: trunk/Source/WebCore/ChangeLog (290821 => 290822)
--- trunk/Source/WebCore/ChangeLog 2022-03-04 08:10:17 UTC (rev 290821)
+++ trunk/Source/WebCore/ChangeLog 2022-03-04 08:42:45 UTC (rev 290822)
@@ -1,3 +1,20 @@
+2022-03-04 Sihui Liu <sihui_...@apple.com>
+
+ SQLiteDatabase::open should return early if journal mode cannot be set
+ https://bugs.webkit.org/show_bug.cgi?id=237130
+ <rdar://83130954>
+
+ Reviewed by Darin Adler.
+
+ Add early return in SQLiteDatabase::open if key operation fails; also make sure error is properly set and
+ database is closed in the case.
+
+ * platform/sql/SQLiteDatabase.cpp:
+ (WebCore::SQLiteDatabase::open):
+ (WebCore::SQLiteDatabase::useWALJournalMode):
+ (WebCore::SQLiteDatabase::close):
+ * platform/sql/SQLiteDatabase.h:
+
2022-03-04 Youenn Fablet <you...@apple.com>
webrtc/canvas-to-peer-connection.html is flakily failing a test assertion
Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp (290821 => 290822)
--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp 2022-03-04 08:10:17 UTC (rev 290821)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp 2022-03-04 08:42:45 UTC (rev 290822)
@@ -38,6 +38,7 @@
#include <thread>
#include <wtf/FileSystem.h>
#include <wtf/Lock.h>
+#include <wtf/Scope.h>
#include <wtf/Threading.h>
#include <wtf/text/CString.h>
#include <wtf/text/StringConcatenateNumbers.h>
@@ -96,9 +97,18 @@
bool SQLiteDatabase::open(const String& filename, OpenMode openMode)
{
initializeSQLiteIfNecessary();
-
close();
+ auto closeDatabase = makeScopeExit([&]() {
+ if (!m_db)
+ return;
+
+ m_openingThread = nullptr;
+ m_openErrorMessage = sqlite3_errmsg(m_db);
+ m_openError = sqlite3_errcode(m_db);
+ close();
+ });
+
{
Locker locker { isDatabaseOpeningForbiddenLock };
if (isDatabaseOpeningForbidden) {
@@ -119,15 +129,17 @@
break;
}
+ int result = SQLITE_OK;
{
SQLiteTransactionInProgressAutoCounter transactionCounter;
- m_openError = sqlite3_open_v2(FileSystem::fileSystemRepresentation(filename).data(), &m_db, flags, nullptr);
+ result = 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());
- close(ShouldSetErrorState::No);
+
+ if (result != SQLITE_OK) {
+ if (!m_db) {
+ m_openError = result;
+ m_openErrorMessage = "sqlite_open returned null";
+ }
return false;
}
}
@@ -134,19 +146,10 @@
overrideUnauthorizedFunctions();
- m_openError = sqlite3_extended_result_codes(m_db, 1);
- if (m_openError != SQLITE_OK) {
- m_openErrorMessage = sqlite3_errmsg(m_db);
- LOG_ERROR("SQLite database error when enabling extended errors - %s", m_openErrorMessage.data());
- close(ShouldSetErrorState::No);
+ m_openingThread = &Thread::current();
+ if (sqlite3_extended_result_codes(m_db, 1) != SQLITE_OK)
return false;
- }
- if (isOpen())
- m_openingThread = &Thread::current();
- else
- m_openErrorMessage = "sqlite_open returned null";
-
{
SQLiteTransactionInProgressAutoCounter transactionCounter;
if (!executeCommand("PRAGMA temp_store = MEMORY;"_s))
@@ -154,19 +157,18 @@
}
if (filename != inMemoryPath()) {
- if (openMode != OpenMode::ReadOnly)
- useWALJournalMode();
+ if (openMode != OpenMode::ReadOnly && !useWALJournalMode())
+ return false;
auto shmFileName = makeString(filename, "-shm"_s);
- if (FileSystem::fileExists(shmFileName)) {
- if (!FileSystem::isSafeToUseMemoryMapForPath(shmFileName)) {
- RELEASE_LOG_FAULT(SQLDatabase, "Opened an SQLite database with a Class A -shm file. This may trigger a crash when the user locks the device. (%s)", shmFileName.latin1().data());
- FileSystem::makeSafeToUseMemoryMapForPath(shmFileName);
- }
+ if (FileSystem::fileExists(shmFileName) && !FileSystem::isSafeToUseMemoryMapForPath(shmFileName)) {
+ RELEASE_LOG_FAULT(SQLDatabase, "Opened an SQLite database with a Class A -shm file. This may trigger a crash when the user locks the device. (%s)", shmFileName.latin1().data());
+ FileSystem::makeSafeToUseMemoryMapForPath(shmFileName);
}
}
- return isOpen();
+ closeDatabase.release();
+ return true;
}
static int walAutomaticTruncationHook(void* context, sqlite3* db, const char* dbName, int walPageCount)
@@ -228,26 +230,35 @@
LOG_ERROR("SQLite database failed to checkpoint: %s", lastErrorMsg());
}
-void SQLiteDatabase::useWALJournalMode()
+bool SQLiteDatabase::useWALJournalMode()
{
m_useWAL = true;
{
SQLiteTransactionInProgressAutoCounter transactionCounter;
auto walStatement = prepareStatement("PRAGMA journal_mode=WAL;"_s);
- if (walStatement && walStatement->step() == SQLITE_ROW) {
+ if (!walStatement)
+ return false;
+
+ int stepResult = walStatement->step();
+ if (stepResult != SQLITE_ROW)
+ return false;
+
#ifndef NDEBUG
- String mode = walStatement->columnText(0);
- if (!equalLettersIgnoringASCIICase(mode, "wal"))
- LOG_ERROR("journal_mode of database should be 'WAL', but is '%s'", mode.utf8().data());
+ String mode = walStatement->columnText(0);
+ if (!equalLettersIgnoringASCIICase(mode, "wal")) {
+ LOG_ERROR("SQLite database journal_mode should be 'WAL', but is '%s'", mode.utf8().data());
+ return false;
+ }
#endif
- } else
- LOG_ERROR("SQLite database failed to set journal_mode to WAL, error: %s", lastErrorMsg());
}
+ // The database can be used even if checkpoint fails, e.g. when there are multiple open database connections.
checkpoint(CheckpointMode::Truncate);
+
+ return true;
}
-void SQLiteDatabase::close(ShouldSetErrorState shouldSetErrorState)
+void SQLiteDatabase::close()
{
if (m_db) {
ASSERT_WITH_MESSAGE(!m_statementCount, "All SQLiteTransaction objects should be destroyed before closing the database");
@@ -271,12 +282,6 @@
if (closeResult != SQLITE_OK)
RELEASE_LOG_ERROR(SQLDatabase, "SQLiteDatabase::close: Failed to close database (%d) - %" PUBLIC_LOG_STRING, closeResult, lastErrorMsg());
}
-
- if (shouldSetErrorState == ShouldSetErrorState::Yes) {
- m_openingThread = nullptr;
- m_openError = SQLITE_ERROR;
- m_openErrorMessage = CString();
- }
}
void SQLiteDatabase::overrideUnauthorizedFunctions()
Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.h (290821 => 290822)
--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.h 2022-03-04 08:10:17 UTC (rev 290821)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.h 2022-03-04 08:42:45 UTC (rev 290822)
@@ -61,8 +61,7 @@
enum class OpenMode { ReadOnly, ReadWrite, ReadWriteCreate };
WEBCORE_EXPORT bool open(const String& filename, OpenMode = OpenMode::ReadWriteCreate);
bool isOpen() const { return m_db; }
- enum class ShouldSetErrorState : bool { No, Yes };
- WEBCORE_EXPORT void close(ShouldSetErrorState = ShouldSetErrorState::Yes);
+ WEBCORE_EXPORT void close();
WEBCORE_EXPORT bool executeCommandSlow(const String&);
WEBCORE_EXPORT bool executeCommand(ASCIILiteral);
@@ -170,7 +169,7 @@
static int authorizerFunction(void*, int, const char*, const char*, const char*, const char*);
void enableAuthorizer(bool enable) WTF_REQUIRES_LOCK(m_authorizerLock);
- void useWALJournalMode();
+ bool useWALJournalMode();
int pageSize();
