Skip to content

Commit

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

    Crash under SQLiteDatabase::~SQLiteDatabase
    https://bugs.webkit.org/show_bug.cgi?id=269648
    rdar://123160407

    Reviewed by David Kilzer.

    The crash was occurring because SQLiteStorageArea::handleDatabaseCorruptionIfNeeded()
    was destroying the SQLiteDatabase object (m_database) but was failing to destroy the
    potential transaction (m_transaction) and cached statements (m_cachedStatements), all
    of which have a CheckedRef pointing to the database.

    Update handleDatabaseCorruptionIfNeeded() to call close(), which clears m_cache,
    m_cacheSize, m_transaction, m_cachedStatements and m_database.

    Also update SQLiteStatement / SQLiteTransaction to keep a WeakRef to the database
    instead of a CheckedRef, as per our updated smart pointer guidelines. Using
    CheckedPtr/CheckedRef for database members is no longer recommended.

    * Source/WebCore/platform/sql/SQLiteDatabase.h:
    * Source/WebCore/platform/sql/SQLiteStatement.h:
    * Source/WebCore/platform/sql/SQLiteTransaction.h:
    * Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp:
    (WebKit::SQLiteStorageArea::handleDatabaseCorruptionIfNeeded):

    Canonical link: https://commits.webkit.org/274937@main

Canonical link: https://commits.webkit.org/274313.39@webkitglib/2.44
  • Loading branch information
cdumez authored and aperezdc committed Mar 8, 2024
1 parent af15dd9 commit 928e57d
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
5 changes: 3 additions & 2 deletions Source/WebCore/platform/sql/SQLiteDatabase.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@

#include <functional>
#include <sqlite3.h>
#include <wtf/CheckedRef.h>
#include <wtf/CheckedPtr.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 +50,7 @@ class DatabaseAuthorizer;
class SQLiteStatement;
class SQLiteTransaction;

class SQLiteDatabase : public CanMakeThreadSafeCheckedPtr {
class SQLiteDatabase : public CanMakeWeakPtr<SQLiteDatabase>, public CanMakeThreadSafeCheckedPtr {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(SQLiteDatabase);
friend class SQLiteTransaction;
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/platform/sql/SQLiteStatement.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#include "SQLValue.h"
#include "SQLiteDatabase.h"
#include <span>
#include <wtf/CheckedRef.h>
#include <wtf/WeakRef.h>

struct sqlite3_stmt;

Expand Down Expand Up @@ -93,7 +93,7 @@ class SQLiteStatement {
template<typename T, typename... Args> bool bindImpl(int i, T first, Args&&... args);
template<typename T> bool bindImpl(int, T);

CheckedRef<SQLiteDatabase> m_database;
WeakRef<SQLiteDatabase> m_database;
sqlite3_stmt* m_statement;
};

Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/platform/sql/SQLiteTransaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
#ifndef SQLiteTransaction_h
#define SQLiteTransaction_h

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

namespace WebCore {

Expand All @@ -51,7 +51,7 @@ class SQLiteTransaction {
SQLiteDatabase& database() const { return m_db.get(); }

private:
CheckedRef<SQLiteDatabase> m_db;
WeakRef<SQLiteDatabase> m_db;
bool m_inProgress;
bool m_readOnly;
};
Expand Down
5 changes: 2 additions & 3 deletions Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,9 +455,8 @@ bool SQLiteStorageArea::handleDatabaseCorruptionIfNeeded(int databaseError)
if (databaseError != SQLITE_CORRUPT && databaseError != SQLITE_NOTADB)
return false;

m_database = nullptr;
m_cache = std::nullopt;
m_cacheSize = std::nullopt;
close();

RELEASE_LOG(Storage, "SQLiteStorageArea::handleDatabaseCorruption deletes corrupted database file '%s'", m_path.utf8().data());
WebCore::SQLiteFileSystem::deleteDatabaseFile(m_path);
return true;
Expand Down

0 comments on commit 928e57d

Please sign in to comment.