- Revision
- 277653
- Author
- [email protected]
- Date
- 2021-05-18 08:35:03 -0700 (Tue, 18 May 2021)
Log Message
Make sure SQLiteStatement objects get destroyed before the database is closed
https://bugs.webkit.org/show_bug.cgi?id=225881
Reviewed by Darin Adler.
Make sure SQLiteStatement objects get destroyed before the database is closed. There are 2 issues
with destroying a SQLiteStatement after a database is closed:
1. The underlying call to close the sqlite database will fail if the database still has statements
and we will leak the database.
2. SQLiteStatement has a reference to the database so it cannot outlive the SQLiteDatabase.
* Modules/webdatabase/DatabaseTracker.cpp:
(WebCore::DatabaseTracker::deleteOrigin):
* platform/sql/SQLiteDatabase.cpp:
(WebCore::SQLiteDatabase::close):
(WebCore::SQLiteDatabase::incrementStatementCount):
(WebCore::SQLiteDatabase::decrementStatementCount):
* platform/sql/SQLiteDatabase.h:
* platform/sql/SQLiteStatement.cpp:
(WebCore::SQLiteStatement::SQLiteStatement):
(WebCore::SQLiteStatement::~SQLiteStatement):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (277652 => 277653)
--- trunk/Source/WebCore/ChangeLog 2021-05-18 14:34:04 UTC (rev 277652)
+++ trunk/Source/WebCore/ChangeLog 2021-05-18 15:35:03 UTC (rev 277653)
@@ -1,3 +1,27 @@
+2021-05-18 Chris Dumez <[email protected]>
+
+ Make sure SQLiteStatement objects get destroyed before the database is closed
+ https://bugs.webkit.org/show_bug.cgi?id=225881
+
+ Reviewed by Darin Adler.
+
+ Make sure SQLiteStatement objects get destroyed before the database is closed. There are 2 issues
+ with destroying a SQLiteStatement after a database is closed:
+ 1. The underlying call to close the sqlite database will fail if the database still has statements
+ and we will leak the database.
+ 2. SQLiteStatement has a reference to the database so it cannot outlive the SQLiteDatabase.
+
+ * Modules/webdatabase/DatabaseTracker.cpp:
+ (WebCore::DatabaseTracker::deleteOrigin):
+ * platform/sql/SQLiteDatabase.cpp:
+ (WebCore::SQLiteDatabase::close):
+ (WebCore::SQLiteDatabase::incrementStatementCount):
+ (WebCore::SQLiteDatabase::decrementStatementCount):
+ * platform/sql/SQLiteDatabase.h:
+ * platform/sql/SQLiteStatement.cpp:
+ (WebCore::SQLiteStatement::SQLiteStatement):
+ (WebCore::SQLiteStatement::~SQLiteStatement):
+
2021-05-18 Philippe Normand <[email protected]>
[MediaStream][GStreamer] Flaky fast/mediastream/MediaStream-video-element-video-tracks-disabled.html
Modified: trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp (277652 => 277653)
--- trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2021-05-18 14:34:04 UTC (rev 277652)
+++ trunk/Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp 2021-05-18 15:35:03 UTC (rev 277653)
@@ -883,30 +883,32 @@
SQLiteTransaction transaction(m_database);
transaction.begin();
- auto statement = m_database.prepareStatement("DELETE FROM Databases WHERE origin=?"_s);
- if (!statement) {
- LOG_ERROR("Unable to prepare deletion of databases from origin %s from tracker", origin.databaseIdentifier().utf8().data());
- return false;
- }
+ {
+ auto statement = m_database.prepareStatement("DELETE FROM Databases WHERE origin=?"_s);
+ if (!statement) {
+ LOG_ERROR("Unable to prepare deletion of databases from origin %s from tracker", origin.databaseIdentifier().utf8().data());
+ return false;
+ }
- statement->bindText(1, origin.databaseIdentifier());
+ statement->bindText(1, origin.databaseIdentifier());
- if (!statement->executeCommand()) {
- LOG_ERROR("Unable to execute deletion of databases from origin %s from tracker", origin.databaseIdentifier().utf8().data());
- return false;
- }
+ if (!statement->executeCommand()) {
+ LOG_ERROR("Unable to execute deletion of databases from origin %s from tracker", origin.databaseIdentifier().utf8().data());
+ return false;
+ }
- auto originStatement = m_database.prepareStatement("DELETE FROM Origins WHERE origin=?"_s);
- if (!originStatement) {
- LOG_ERROR("Unable to prepare deletion of origin %s from tracker", origin.databaseIdentifier().utf8().data());
- return false;
- }
+ auto originStatement = m_database.prepareStatement("DELETE FROM Origins WHERE origin=?"_s);
+ if (!originStatement) {
+ LOG_ERROR("Unable to prepare deletion of origin %s from tracker", origin.databaseIdentifier().utf8().data());
+ return false;
+ }
- originStatement->bindText(1, origin.databaseIdentifier());
+ originStatement->bindText(1, origin.databaseIdentifier());
- if (!originStatement->executeCommand()) {
- LOG_ERROR("Unable to execute deletion of databases from origin %s from tracker", origin.databaseIdentifier().utf8().data());
- return false;
+ if (!originStatement->executeCommand()) {
+ LOG_ERROR("Unable to execute deletion of databases from origin %s from tracker", origin.databaseIdentifier().utf8().data());
+ return false;
+ }
}
transaction.commit();
Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp (277652 => 277653)
--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp 2021-05-18 14:34:04 UTC (rev 277652)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp 2021-05-18 15:35:03 UTC (rev 277653)
@@ -222,6 +222,8 @@
void SQLiteDatabase::close()
{
if (m_db) {
+ ASSERT_WITH_MESSAGE(!m_statementCount, "All SQLiteTransaction objects should be destroyed before closing the database");
+
// FIXME: This is being called on the main thread during JS GC. <rdar://problem/5739818>
// ASSERT(m_openingThread == &Thread::current());
sqlite3* db = m_db;
Modified: trunk/Source/WebCore/platform/sql/SQLiteDatabase.h (277652 => 277653)
--- trunk/Source/WebCore/platform/sql/SQLiteDatabase.h 2021-05-18 14:34:04 UTC (rev 277652)
+++ trunk/Source/WebCore/platform/sql/SQLiteDatabase.h 2021-05-18 15:35:03 UTC (rev 277653)
@@ -32,6 +32,7 @@
#include <wtf/Lock.h>
#include <wtf/Threading.h>
#include <wtf/UniqueRef.h>
+#include <wtf/WeakPtr.h>
#include <wtf/text/CString.h>
#include <wtf/text/WTFString.h>
@@ -47,7 +48,7 @@
class SQLiteStatement;
class SQLiteTransaction;
-class SQLiteDatabase {
+class SQLiteDatabase : public CanMakeWeakPtr<SQLiteDatabase> {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(SQLiteDatabase);
friend class SQLiteTransaction;
@@ -155,6 +156,9 @@
WEBCORE_EXPORT void releaseMemory();
+ void incrementStatementCount();
+ void decrementStatementCount();
+
private:
static int authorizerFunction(void*, int, const char*, const char*, const char*, const char*);
@@ -171,6 +175,7 @@
bool m_transactionInProgress { false };
#if ASSERT_ENABLED
bool m_sharable { false };
+ std::atomic<unsigned> m_statementCount { 0 };
#endif
bool m_useWAL { false };
@@ -187,4 +192,19 @@
CString m_openErrorMessage;
};
+inline void SQLiteDatabase::incrementStatementCount()
+{
+#if ASSERT_ENABLED
+ ++m_statementCount;
+#endif
+}
+
+inline void SQLiteDatabase::decrementStatementCount()
+{
+#if ASSERT_ENABLED
+ ASSERT(m_statementCount);
+ --m_statementCount;
+#endif
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/sql/SQLiteStatement.cpp (277652 => 277653)
--- trunk/Source/WebCore/platform/sql/SQLiteStatement.cpp 2021-05-18 14:34:04 UTC (rev 277652)
+++ trunk/Source/WebCore/platform/sql/SQLiteStatement.cpp 2021-05-18 15:35:03 UTC (rev 277653)
@@ -46,6 +46,7 @@
, m_statement(statement)
{
ASSERT(statement);
+ m_database.incrementStatementCount();
}
SQLiteStatement::SQLiteStatement(SQLiteStatement&& other)
@@ -52,11 +53,13 @@
: m_database(other.database())
, m_statement(std::exchange(other.m_statement, nullptr))
{
+ m_database.incrementStatementCount();
}
SQLiteStatement::~SQLiteStatement()
{
sqlite3_finalize(m_statement);
+ m_database.decrementStatementCount();
}
int SQLiteStatement::step()
Modified: trunk/Source/WebKitLegacy/Storage/StorageTracker.cpp (277652 => 277653)
--- trunk/Source/WebKitLegacy/Storage/StorageTracker.cpp 2021-05-18 14:34:04 UTC (rev 277652)
+++ trunk/Source/WebKitLegacy/Storage/StorageTracker.cpp 2021-05-18 15:35:03 UTC (rev 277653)
@@ -519,16 +519,18 @@
return;
}
- auto deleteStatement = m_database.prepareStatement("DELETE FROM Origins where origin=?"_s);
- if (!deleteStatement) {
- LOG_ERROR("Unable to prepare deletion of origin '%s'", originIdentifier.ascii().data());
- return;
+ {
+ auto deleteStatement = m_database.prepareStatement("DELETE FROM Origins where origin=?"_s);
+ if (!deleteStatement) {
+ LOG_ERROR("Unable to prepare deletion of origin '%s'", originIdentifier.ascii().data());
+ return;
+ }
+ deleteStatement->bindText(1, originIdentifier);
+ if (!deleteStatement->executeCommand()) {
+ LOG_ERROR("Unable to execute deletion of origin '%s'", originIdentifier.ascii().data());
+ return;
+ }
}
- deleteStatement->bindText(1, originIdentifier);
- if (!deleteStatement->executeCommand()) {
- LOG_ERROR("Unable to execute deletion of origin '%s'", originIdentifier.ascii().data());
- return;
- }
FileSystem::deleteFile(path);
