Skip to content

Commit 76d3b46

Browse files
[LocalStorage] Fix NetworkProcess crash on sqlite database corruption
1) Corrupted database is handled inside handleDatabaseCorruptionIfNeeded(), that closes database connection and clears all cached statements (m_cachedStatements). The second destroys all SQLiteStatement(s) objects. But a pointer to such object may be still kept inside SQLiteStatementAutoResetScope created inside SQLiteStorageArea.cpp: auto statement = cachedStatement(StatementType::SetItem); This will call m_statement->reset() at scope exit (from ~SQLiteStatementAutoResetScope) on already deleted object that results with crash. The solution is to use WeakPtr to make sure object is still alive before calling statement->reset() 2) Special case happens when corruption is detected on setting new item (setItem). If there are no cached entries, handleDatabaseCorruptionIfNeeded() will not create new database (m_database == nullptr) that will lead to another crash when re-trying to create cachedStatement again. Solution here is to replace direct statement execution with full setItem() call that will prepareDatabase() again if needed.
1 parent 3fc6679 commit 76d3b46

4 files changed

Lines changed: 14 additions & 8 deletions

File tree

Source/WebCore/platform/sql/SQLiteStatement.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,13 @@
2828
#include "SQLValue.h"
2929
#include "SQLiteDatabase.h"
3030
#include <wtf/Span.h>
31+
#include <wtf/WeakPtr.h>
3132

3233
struct sqlite3_stmt;
3334

3435
namespace WebCore {
3536

36-
class SQLiteStatement {
37+
class SQLiteStatement : public CanMakeWeakPtr<SQLiteStatement> {
3738
WTF_MAKE_NONCOPYABLE(SQLiteStatement); WTF_MAKE_FAST_ALLOCATED;
3839
public:
3940
WEBCORE_EXPORT ~SQLiteStatement();

Source/WebCore/platform/sql/SQLiteStatementAutoResetScope.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,7 @@ SQLiteStatementAutoResetScope::~SQLiteStatementAutoResetScope()
5454
m_statement->reset();
5555
}
5656

57+
SQLiteStatement* SQLiteStatementAutoResetScope::get() { return m_statement.get(); }
58+
SQLiteStatement* SQLiteStatementAutoResetScope::operator->() { return m_statement.get(); }
59+
5760
}

Source/WebCore/platform/sql/SQLiteStatementAutoResetScope.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include <wtf/FastMalloc.h>
2929
#include <wtf/Noncopyable.h>
30+
#include <wtf/WeakPtr.h>
3031

3132
namespace WebCore {
3233
class SQLiteStatement;
@@ -39,14 +40,14 @@ class SQLiteStatementAutoResetScope {
3940
WEBCORE_EXPORT SQLiteStatementAutoResetScope& operator=(SQLiteStatementAutoResetScope&& other);
4041
WEBCORE_EXPORT ~SQLiteStatementAutoResetScope();
4142

42-
explicit operator bool() const { return m_statement; }
43+
explicit operator bool() const { return !!m_statement; }
4344
bool operator!() const { return !m_statement; }
4445

45-
SQLiteStatement* get() { return m_statement; }
46-
SQLiteStatement* operator->() { return m_statement; }
46+
SQLiteStatement* get();
47+
SQLiteStatement* operator->();
4748

4849
private:
49-
SQLiteStatement* m_statement;
50+
WeakPtr<SQLiteStatement> m_statement;
5051
};
5152

5253
} // namespace WebCore

Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,10 @@ Expected<void, StorageError> SQLiteStorageArea::setItem(const String& key, const
356356
if (result != SQLITE_DONE) {
357357
if (!handleDatabaseCorruption || !handleDatabaseCorruptionIfNeeded(result))
358358
return makeUnexpected(StorageError::Database);
359-
statement = cachedStatement(StatementType::SetItem);
360-
if (!statement || statement->bindText(1, key) || statement->bindBlob(2, value) || statement->step() != SQLITE_DONE)
361-
return makeUnexpected(StorageError::Database);
359+
360+
// The database was recovered from corruption.
361+
// Try again but without handling database corruption this time to avoid infinite loop.
362+
return setItem(key, value, false);
362363
}
363364

364365
updateCacheIfNeeded(key, value);

0 commit comments

Comments
 (0)