Fix a bug that could lead to a crash. Some parts of
authordumi@chromium.org <dumi@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Mar 2010 01:01:23 +0000 (01:01 +0000)
committerdumi@chromium.org <dumi@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Mar 2010 01:01:23 +0000 (01:01 +0000)
SQLTransaction::checkAndHandleClosedDatabase() should only be run
when that method is invoked on the DB thread.

Reviewed by Dimitri Glazkov.

We cannot test this fix with a test, because the crash happens
only when all of the following conditions are met:
1. A database is closing.
2. A transaction on that database is in progress.
3. The transaction is in a state where a statement/transaction
success/error callback needs to be invoked (so there's a task for
this transaction pending on the main thread).
4. The DB thread finished processing all its tasks and called
SQLTransactionCoordinator::shutdown() before the main thread go to
that task.

The closest thing we have to a test is running
LayoutTests/storage/database-lock-after-reload.html 1000 times in
a row. Without the patch, the probability of a crash happening in
one of the runs is very high. With the patch, the test should
reliably run 1000 times in a row without a single crash.

https://bugs.webkit.org/show_bug.cgi?id=35624

* storage/SQLTransaction.cpp:
(WebCore::SQLTransaction::checkAndHandleClosedDatabase):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@55487 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/storage/SQLTransaction.cpp

index 8ff8eb5100cce05337500fef60a6ec563c81d006..f0dc2364b8f2e0669c120c309736290f6c06d682 100644 (file)
@@ -1,3 +1,33 @@
+2010-03-02  Dumitru Daniliuc  <dumi@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Fix a bug that could lead to a crash. Some parts of
+        SQLTransaction::checkAndHandleClosedDatabase() should only be run
+        when that method is invoked on the DB thread.
+
+        We cannot test this fix with a test, because the crash happens
+        only when all of the following conditions are met:
+        1. A database is closing.
+        2. A transaction on that database is in progress.
+        3. The transaction is in a state where a statement/transaction
+        success/error callback needs to be invoked (so there's a task for
+        this transaction pending on the main thread).
+        4. The DB thread finished processing all its tasks and called
+        SQLTransactionCoordinator::shutdown() before the main thread go to
+        that task.
+
+        The closest thing we have to a test is running
+        LayoutTests/storage/database-lock-after-reload.html 1000 times in
+        a row. Without the patch, the probability of a crash happening in
+        one of the runs is very high. With the patch, the test should
+        reliably run 1000 times in a row without a single crash.
+
+        https://bugs.webkit.org/show_bug.cgi?id=35624
+
+        * storage/SQLTransaction.cpp:
+        (WebCore::SQLTransaction::checkAndHandleClosedDatabase):
+
 2010-03-03  Darin Fisher  <darin@chromium.org>
 
         Reviewed by Mark Rowe.
index 754cebc3a91c0543519680ca30924a11f2d86140..a7c25587549f33f369fdf1edd771f8af8f6f66d9 100644 (file)
@@ -158,6 +158,10 @@ void SQLTransaction::checkAndHandleClosedDatabase()
     m_statementQueue.clear();
     m_nextStep = 0;
 
+    // The next steps should be executed only if we're on the DB thread.
+    if (currentThread() != database()->scriptExecutionContext()->databaseThread()->getThreadID())
+        return;
+
     // The current SQLite transaction should be stopped, as well
     if (m_sqliteTransaction) {
         m_sqliteTransaction->stop();