Skip to content

Commit

Permalink
Cherry-pick 269697@main (9405d3e). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history 
…gi?id=263539

    Use a CheckedRef for SQLiteTransaction::m_db
    https://bugs.webkit.org/show_bug.cgi?id=263539
    rdar://117364281

    Reviewed by Sihui Liu and Chris Dumez.

    Also remove `CanMakeWeakPtr<SQLiteDatabase>` since it's not used.

    * Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp:
    (WebCore::IDBServer::SQLiteIDBCursor::createSQLiteStatement):
    (WebCore::IDBServer::SQLiteIDBCursor::resetAndRebindPreIndexStatementIfNecessary):
    (WebCore::IDBServer::SQLiteIDBCursor::internalFetchNextRecord):
    * Source/WebCore/platform/sql/SQLiteDatabase.h:
    * Source/WebCore/platform/sql/SQLiteTransaction.cpp:
    (WebCore::SQLiteTransaction::begin):
    (WebCore::SQLiteTransaction::commit):
    (WebCore::SQLiteTransaction::rollback):
    (WebCore::SQLiteTransaction::stop):
    (WebCore::SQLiteTransaction::wasRolledBackBySqlite const):
    * Source/WebCore/platform/sql/SQLiteTransaction.h:
    (WebCore::SQLiteTransaction::database const):

    Canonical link: https://commits.webkit.org/269697@main
  • Loading branch information
@charliewolfe @aperezdc
charliewolfe authored and aperezdc committed Mar 14, 2024
1 parent 0264501 commit 3705e6a
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 30 deletions.
25 changes: 13 additions & 12 deletions Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,10 @@ bool SQLiteIDBCursor::createSQLiteStatement(StringView sql)
ASSERT(!m_currentUpperKey.isNull());
ASSERT(m_transaction->sqliteTransaction());

auto statement = m_transaction->sqliteTransaction()->database().prepareHeapStatementSlow(sql);
CheckedRef database = m_transaction->sqliteTransaction()->database();
auto statement = database->prepareHeapStatementSlow(sql);
if (!statement) {
LOG_ERROR("Could not create cursor statement (prepare/id) - '%s'", m_transaction->sqliteTransaction()->database().lastErrorMsg());
LOG_ERROR("Could not create cursor statement (prepare/id) - '%s'", database->lastErrorMsg());
return false;
}
m_statement = statement.value().moveToUniquePtr();
Expand Down Expand Up @@ -293,18 +294,18 @@ bool SQLiteIDBCursor::resetAndRebindPreIndexStatementIfNecessary()
if (m_currentIndexRecordValue.isNull())
return true;

auto& database = m_transaction->sqliteTransaction()->database();
CheckedRef database = m_transaction->sqliteTransaction()->database();
if (!m_preIndexStatement) {
auto preIndexStatement = database.prepareHeapStatementSlow(buildPreIndexStatement(isDirectionNext()));
auto preIndexStatement = database->prepareHeapStatementSlow(buildPreIndexStatement(isDirectionNext()));
if (!preIndexStatement) {
LOG_ERROR("Could not prepare pre statement - '%s'", database.lastErrorMsg());
LOG_ERROR("Could not prepare pre statement - '%s'", database->lastErrorMsg());
return false;
}
m_preIndexStatement = preIndexStatement.value().moveToUniquePtr();
}

if (m_preIndexStatement->reset() != SQLITE_OK) {
LOG_ERROR("Could not reset pre statement - '%s'", database.lastErrorMsg());
LOG_ERROR("Could not reset pre statement - '%s'", database->lastErrorMsg());
return false;
}

Expand Down Expand Up @@ -477,7 +478,7 @@ SQLiteIDBCursor::FetchResult SQLiteIDBCursor::internalFetchNextRecord(SQLiteCurs

record.record.value = { };

auto& database = m_transaction->sqliteTransaction()->database();
CheckedRef database = m_transaction->sqliteTransaction()->database();
SQLiteStatement* statement = nullptr;

int result;
Expand All @@ -488,7 +489,7 @@ SQLiteIDBCursor::FetchResult SQLiteIDBCursor::internalFetchNextRecord(SQLiteCurs
if (result == SQLITE_ROW)
statement = m_preIndexStatement.get();
else if (result != SQLITE_DONE)
LOG_ERROR("Error advancing with pre statement - (%i) %s", result, database.lastErrorMsg());
LOG_ERROR("Error advancing with pre statement - (%i) %s", result, database->lastErrorMsg());
}

if (!statement) {
Expand All @@ -499,7 +500,7 @@ SQLiteIDBCursor::FetchResult SQLiteIDBCursor::internalFetchNextRecord(SQLiteCurs
return FetchResult::Success;
}
if (result != SQLITE_ROW) {
LOG_ERROR("Error advancing cursor - (%i) %s", result, database.lastErrorMsg());
LOG_ERROR("Error advancing cursor - (%i) %s", result, database->lastErrorMsg());
markAsErrored(record);
return FetchResult::Failure;
}
Expand Down Expand Up @@ -540,14 +541,14 @@ SQLiteIDBCursor::FetchResult SQLiteIDBCursor::internalFetchNextRecord(SQLiteCurs
}

if (!m_cachedObjectStoreStatement || m_cachedObjectStoreStatement->reset() != SQLITE_OK) {
if (auto cachedObjectStoreStatement = database.prepareHeapStatement("SELECT value FROM Records WHERE key = CAST(? AS TEXT) and objectStoreID = ?;"_s))
if (auto cachedObjectStoreStatement = database->prepareHeapStatement("SELECT value FROM Records WHERE key = CAST(? AS TEXT) and objectStoreID = ?;"_s))
m_cachedObjectStoreStatement = cachedObjectStoreStatement.value().moveToUniquePtr();
}

if (!m_cachedObjectStoreStatement
|| m_cachedObjectStoreStatement->bindBlob(1, keyData) != SQLITE_OK
|| m_cachedObjectStoreStatement->bindInt64(2, m_objectStoreID) != SQLITE_OK) {
LOG_ERROR("Could not create index cursor statement into object store records (%i) '%s'", database.lastError(), database.lastErrorMsg());
LOG_ERROR("Could not create index cursor statement into object store records (%i) '%s'", database->lastError(), database->lastErrorMsg());
markAsErrored(record);
return FetchResult::Failure;
}
Expand All @@ -561,7 +562,7 @@ SQLiteIDBCursor::FetchResult SQLiteIDBCursor::internalFetchNextRecord(SQLiteCurs
// Skip over it.
return FetchResult::ShouldFetchAgain;
} else {
LOG_ERROR("Could not step index cursor statement into object store records (%i) '%s'", database.lastError(), database.lastErrorMsg());
LOG_ERROR("Could not step index cursor statement into object store records (%i) '%s'", database->lastError(), database->lastErrorMsg());
markAsErrored(record);
return FetchResult::Failure;

Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/platform/sql/SQLiteDatabase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@

#include <functional>
#include <sqlite3.h>
#include <wtf/CheckedRef.h>
#include <wtf/Expected.h>
#include <wtf/Lock.h>
#include <wtf/OptionSet.h>
#include <wtf/Threading.h>
#include <wtf/UniqueRef.h>
#include <wtf/WeakPtr.h>
#include <wtf/text/CString.h>
#include <wtf/text/WTFString.h>

Expand All @@ -49,7 +49,7 @@ class DatabaseAuthorizer;
class SQLiteStatement;
class SQLiteTransaction;

class SQLiteDatabase : public CanMakeWeakPtr<SQLiteDatabase> {
class SQLiteDatabase : public CanMakeThreadSafeCheckedPtr {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(SQLiteDatabase);
friend class SQLiteTransaction;
Expand Down
28 changes: 14 additions & 14 deletions Source/WebCore/platform/sql/SQLiteTransaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ SQLiteTransaction::~SQLiteTransaction()
void SQLiteTransaction::begin()
{
if (!m_inProgress) {
ASSERT(!m_db.m_transactionInProgress);
ASSERT(!m_db->m_transactionInProgress);
// Call BEGIN IMMEDIATE for a write transaction to acquire
// a RESERVED lock on the DB file. Otherwise, another write
// transaction (on another connection) could make changes
Expand All @@ -59,14 +59,14 @@ void SQLiteTransaction::begin()
SQLiteDatabaseTracker::incrementTransactionInProgressCount();
int result = SQLITE_OK;
if (m_readOnly)
result = m_db.execute("BEGIN"_s);
result = m_db->execute("BEGIN"_s);
else
result = m_db.execute("BEGIN IMMEDIATE"_s);
result = m_db->execute("BEGIN IMMEDIATE"_s);
if (result == SQLITE_DONE)
m_inProgress = true;
else
RELEASE_LOG_ERROR(SQLDatabase, "SQLiteTransaction::begin: Failed to begin transaction (error %d)", result);
m_db.m_transactionInProgress = m_inProgress;
m_db->m_transactionInProgress = m_inProgress;
if (!m_inProgress)
SQLiteDatabaseTracker::decrementTransactionInProgressCount();
} else
Expand All @@ -76,25 +76,25 @@ void SQLiteTransaction::begin()
void SQLiteTransaction::commit()
{
if (m_inProgress) {
ASSERT(m_db.m_transactionInProgress);
m_inProgress = !m_db.executeCommand("COMMIT"_s);
m_db.m_transactionInProgress = m_inProgress;
ASSERT(m_db->m_transactionInProgress);
m_inProgress = !m_db->executeCommand("COMMIT"_s);
m_db->m_transactionInProgress = m_inProgress;
if (!m_inProgress)
SQLiteDatabaseTracker::decrementTransactionInProgressCount();
}
}

void SQLiteTransaction::rollback()
{
// We do not use the 'm_inProgress = m_db.executeCommand("ROLLBACK")' construct here,
// We do not use the 'm_inProgress = m_db->executeCommand("ROLLBACK")' construct here,
// because m_inProgress should always be set to false after a ROLLBACK, and
// m_db.executeCommand("ROLLBACK") can sometimes harmlessly fail, thus returning
// m_db->executeCommand("ROLLBACK") can sometimes harmlessly fail, thus returning
// a non-zero/true result (http://www.sqlite.org/lang_transaction.html).
if (m_inProgress) {
ASSERT(m_db.m_transactionInProgress);
m_db.executeCommand("ROLLBACK"_s);
ASSERT(m_db->m_transactionInProgress);
m_db->executeCommand("ROLLBACK"_s);
m_inProgress = false;
m_db.m_transactionInProgress = false;
m_db->m_transactionInProgress = false;
SQLiteDatabaseTracker::decrementTransactionInProgressCount();
}
}
Expand All @@ -103,7 +103,7 @@ void SQLiteTransaction::stop()
{
if (m_inProgress) {
m_inProgress = false;
m_db.m_transactionInProgress = false;
m_db->m_transactionInProgress = false;
SQLiteDatabaseTracker::decrementTransactionInProgressCount();
}
}
Expand All @@ -112,7 +112,7 @@ bool SQLiteTransaction::wasRolledBackBySqlite() const
{
// According to http://www.sqlite.org/c3ref/get_autocommit.html,
// the auto-commit flag should be off in the middle of a transaction
return m_inProgress && m_db.isAutoCommitOn();
return m_inProgress && m_db->isAutoCommitOn();
}

} // namespace WebCore
5 changes: 3 additions & 2 deletions Source/WebCore/platform/sql/SQLiteTransaction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#ifndef SQLiteTransaction_h
#define SQLiteTransaction_h

#include <wtf/CheckedRef.h>
#include <wtf/FastMalloc.h>
#include <wtf/Noncopyable.h>

Expand All @@ -47,10 +48,10 @@ class SQLiteTransaction {
bool inProgress() const { return m_inProgress; }
WEBCORE_EXPORT bool wasRolledBackBySqlite() const;

SQLiteDatabase& database() const { return m_db; }
SQLiteDatabase& database() { return m_db.get(); }

private:
SQLiteDatabase& m_db;
CheckedRef<SQLiteDatabase> m_db;
bool m_inProgress;
bool m_readOnly;
};
Expand Down

0 comments on commit 3705e6a

Please sign in to comment.