Skip to content

Commit

Permalink
Close database connection in SQLiteStorageArea on SQLITE_IOERR
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=270940
rdar://123797002

Reviewed by Chris Dumez.

Database connection is dead after SQLITE_IOERR errors. SQLiteStorageArea should close and re-open database so it has a
chance to recover on such errors.

* Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp:
(WebKit::SQLiteStorageArea::isEmpty):
(WebKit::SQLiteStorageArea::prepareDatabase):
(WebKit::SQLiteStorageArea::getItemFromDatabase):
(WebKit::SQLiteStorageArea::allItems):
(WebKit::SQLiteStorageArea::setItem):
(WebKit::SQLiteStorageArea::removeItem):
(WebKit::SQLiteStorageArea::clear):
(WebKit::SQLiteStorageArea::handleDatabaseErrorIfNeeded):
(WebKit::SQLiteStorageArea::handleDatabaseCorruptionIfNeeded): Deleted.
* Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.h:

Canonical link: https://commits.webkit.org/276114@main
  • Loading branch information
@szewai
szewai committed Mar 14, 2024
1 parent ac5eb11 commit 7665402
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 19 deletions.
42 changes: 24 additions & 18 deletions Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ bool SQLiteStorageArea::isEmpty()

auto statement = cachedStatement(StatementType::CountItems);
if (!statement || statement->step() != SQLITE_ROW) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::isEmpty failed on executing statement (%d) - %s", m_database->lastError(), m_database->lastErrorMsg());
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::isEmpty failed on executing statement (%d) - %{%public}s", m_database->lastError(), m_database->lastErrorMsg());
return true;
}

Expand Down Expand Up @@ -166,7 +166,7 @@ bool SQLiteStorageArea::prepareDatabase(ShouldCreateIfNotExists shouldCreateIfNo
m_database = makeUnique<WebCore::SQLiteDatabase>();
FileSystem::makeAllDirectories(FileSystem::parentPath(m_path));
auto openResult = m_database->open(m_path, WebCore::SQLiteDatabase::OpenMode::ReadWriteCreate, WebCore::SQLiteDatabase::OpenOptions::CanSuspendWhileLocked);
if (!openResult && handleDatabaseCorruptionIfNeeded(m_database->lastError())) {
if (!openResult && handleDatabaseErrorIfNeeded(m_database->lastError()) == IsDatabaseDeleted::Yes) {
databaseExists = false;
if (shouldCreateIfNotExists == ShouldCreateIfNotExists::No)
return true;
Expand Down Expand Up @@ -264,7 +264,7 @@ Expected<String, StorageError> SQLiteStorageArea::getItemFromDatabase(const Stri
return statement->columnBlobAsString(0);
if (result != SQLITE_DONE) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::getItemFromDatabase failed on stepping statement (%d) - %{public}s", m_database->lastError(), m_database->lastErrorMsg());
handleDatabaseCorruptionIfNeeded(result);
handleDatabaseErrorIfNeeded(result);

return makeUnexpected(StorageError::Database);
}
Expand Down Expand Up @@ -318,7 +318,7 @@ HashMap<String, String> SQLiteStorageArea::allItems()

if (result != SQLITE_DONE) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::allItems failed on executing statement (%d) - %{public}s", m_database->lastError(), m_database->lastErrorMsg());
handleDatabaseCorruptionIfNeeded(result);
handleDatabaseErrorIfNeeded(result);
}

return items;
Expand Down Expand Up @@ -348,7 +348,7 @@ Expected<void, StorageError> SQLiteStorageArea::setItem(IPC::Connection::UniqueI
const auto result = statement->step();
if (result != SQLITE_DONE) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::setItem failed on stepping statement (%d) - %{public}s", m_database->lastError(), m_database->lastErrorMsg());
handleDatabaseCorruptionIfNeeded(result);
handleDatabaseErrorIfNeeded(result);
return makeUnexpected(StorageError::Database);
}

Expand Down Expand Up @@ -377,14 +377,14 @@ Expected<void, StorageError> SQLiteStorageArea::removeItem(IPC::Connection::Uniq

auto statement = cachedStatement(StatementType::DeleteItem);
if (!statement || statement->bindText(1, key)) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::removeItem failed on creating statement (%d) - %s", m_database->lastError(), m_database->lastErrorMsg());
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::removeItem failed on creating statement (%d) - %{public}s", m_database->lastError(), m_database->lastErrorMsg());
return makeUnexpected(StorageError::Database);
}

const auto result = statement->step();
if (result != SQLITE_DONE) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::removeItem failed on executing statement (%d) - %s", m_database->lastError(), m_database->lastErrorMsg());
handleDatabaseCorruptionIfNeeded(result);
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::removeItem failed on executing statement (%d) - %{public}s", m_database->lastError(), m_database->lastErrorMsg());
handleDatabaseErrorIfNeeded(result);

return makeUnexpected(StorageError::Database);
}
Expand Down Expand Up @@ -416,14 +416,14 @@ Expected<void, StorageError> SQLiteStorageArea::clear(IPC::Connection::UniqueID
startTransactionIfNecessary();
auto statement = cachedStatement(StatementType::DeleteAllItems);
if (!statement) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::clear failed on creating statement (%d) - %s", m_database->lastError(), m_database->lastErrorMsg());
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::clear failed on creating statement (%d) - %{public}s", m_database->lastError(), m_database->lastErrorMsg());
return makeUnexpected(StorageError::Database);
}

const auto result = statement->step();
if (result != SQLITE_DONE) {
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::clear failed on executing statement (%d) - %s", m_database->lastError(), m_database->lastErrorMsg());
handleDatabaseCorruptionIfNeeded(result);
RELEASE_LOG_ERROR(Storage, "SQLiteStorageArea::clear failed on executing statement (%d) - %{public}s", m_database->lastError(), m_database->lastErrorMsg());
handleDatabaseErrorIfNeeded(result);

return makeUnexpected(StorageError::Database);
}
Expand All @@ -450,16 +450,22 @@ void SQLiteStorageArea::handleLowMemoryWarning()
m_database->releaseMemory();
}

bool SQLiteStorageArea::handleDatabaseCorruptionIfNeeded(int databaseError)
SQLiteStorageArea::IsDatabaseDeleted SQLiteStorageArea::handleDatabaseErrorIfNeeded(int databaseError)
{
if (databaseError != SQLITE_CORRUPT && databaseError != SQLITE_NOTADB)
return false;
if ((databaseError & 0XFF) == SQLITE_IOERR) {
close();
return IsDatabaseDeleted::No;
}

close();
if (databaseError == SQLITE_CORRUPT || databaseError == SQLITE_NOTADB) {
close();

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

return IsDatabaseDeleted::No;
}

void SQLiteStorageArea::updateCacheIfNeeded(const String& key, const String& value)
Expand Down
3 changes: 2 additions & 1 deletion Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class SQLiteStorageArea final : public StorageAreaBase {
WebCore::SQLiteStatementAutoResetScope cachedStatement(StatementType);
Expected<String, StorageError> getItem(const String& key);
Expected<String, StorageError> getItemFromDatabase(const String& key);
bool handleDatabaseCorruptionIfNeeded(int databaseError);
enum class IsDatabaseDeleted : bool { No, Yes };
IsDatabaseDeleted handleDatabaseErrorIfNeeded(int databaseError);
void updateCacheIfNeeded(const String& key, const String& value);
bool requestSpace(const String& key, const String& value);

Expand Down

0 comments on commit 7665402

Please sign in to comment.