Changeset 47170 in webkit


Ignore:
Timestamp:
Aug 12, 2009 5:52:54 PM (15 years ago)
Author:
eric@webkit.org
Message:

2009-08-12 Dumitru Daniliuc <​dumi@chromium.org>

Reviewed by Eric Seidel.

Regression test. Tests that two transactions that run on two
different handles for the same DB do not cause a deadlock.

​https://bugs.webkit.org/show_bug?id=27966

  • storage/multiple-transactions-on-different-handles-expected.txt: Added.
  • storage/multiple-transactions-on-different-handles.html: Added.

2009-08-12 Dumitru Daniliuc <​dumi@chromium.org>

Reviewed by Eric Seidel.

Fixing a deadlock caused by two transactions that run on two
different database handles for the same DB. Adding a per-DB thread
transaction coordinator that allows the DB thread to run only one
transaction per DB file at any given time.

Adding a regression test for this bug.

Test: storage/multiple-transactions-on-different-handles.html

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

  • GNUmakefile.am:
  • WebCore.gypi:
  • WebCore.vcproj/WebCore.vcproj:
  • WebCore.xcodeproj/project.pbxproj:
  • storage/Database.cpp: (WebCore::Database::transactionCoordinator):
  • storage/Database.h:
  • storage/DatabaseThread.cpp: (WebCore::DatabaseThread::DatabaseThread):
  • storage/DatabaseThread.h: (WebCore::DatabaseThread::transactionCoordinator):
  • storage/SQLTransaction.cpp: (WebCore::SQLTransaction::SQLTransaction): (WebCore::SQLTransaction::debugStepName): (WebCore::SQLTransaction::performNextStep): (WebCore::SQLTransaction::aquireLock): (WebCore::SQLTransaction::lockAquired): (WebCore::SQLTransaction::cleanupAfterSuccessCallback): (WebCore::SQLTransaction::cleanupAfterTransactionErrorCallback):
  • storage/SQLTransaction.h:
  • storage/SQLTransactionCoordinator.cpp: Added.
  • storage/SQLTransactionCoordinator.h: Added.
Location:
trunk
Files:
4 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r47169 r47170  
     12009-08-12  Dumitru Daniliuc  <dumi@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Regression test. Tests that two transactions that run on two
     6        different handles for the same DB do not cause a deadlock.
     7
     8        https://bugs.webkit.org/show_bug?id=27966
     9
     10        * storage/multiple-transactions-on-different-handles-expected.txt: Added.
     11        * storage/multiple-transactions-on-different-handles.html: Added.
     12
    1132009-08-12  Eric Seidel  <eric@webkit.org>
    214

  • trunk/WebCore/ChangeLog

    r47165 r47170  
     12009-08-12  Dumitru Daniliuc  <dumi@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Fixing a deadlock caused by two transactions that run on two
     6        different database handles for the same DB. Adding a per-DB thread
     7        transaction coordinator that allows the DB thread to run only one
     8        transaction per DB file at any given time.
     9
     10        Adding a regression test for this bug.
     11
     12        Test: storage/multiple-transactions-on-different-handles.html
     13
     14        https://bugs.webkit.org/show_bug.cgi?id=27996
     15
     16        * GNUmakefile.am:
     17        * WebCore.gypi:
     18        * WebCore.vcproj/WebCore.vcproj:
     19        * WebCore.xcodeproj/project.pbxproj:
     20        * storage/Database.cpp:
     21        (WebCore::Database::transactionCoordinator):
     22        * storage/Database.h:
     23        * storage/DatabaseThread.cpp:
     24        (WebCore::DatabaseThread::DatabaseThread):
     25        * storage/DatabaseThread.h:
     26        (WebCore::DatabaseThread::transactionCoordinator):
     27        * storage/SQLTransaction.cpp:
     28        (WebCore::SQLTransaction::SQLTransaction):
     29        (WebCore::SQLTransaction::debugStepName):
     30        (WebCore::SQLTransaction::performNextStep):
     31        (WebCore::SQLTransaction::aquireLock):
     32        (WebCore::SQLTransaction::lockAquired):
     33        (WebCore::SQLTransaction::cleanupAfterSuccessCallback):
     34        (WebCore::SQLTransaction::cleanupAfterTransactionErrorCallback):
     35        * storage/SQLTransaction.h:
     36        * storage/SQLTransactionCoordinator.cpp: Added.
     37        * storage/SQLTransactionCoordinator.h: Added.
     38
    1392009-08-12  Darin Adler  <darin@apple.com>
    240

  • trunk/WebCore/GNUmakefile.am

    r47165 r47170  
    21172117        WebCore/storage/SQLTransaction.h \
    21182118        WebCore/storage/SQLTransactionCallback.h \
     2119        WebCore/storage/SQLTransactionCoordinator.h \
     2120        WebCore/storage/SQLTransactionCoordinator.cpp \
    21192121        WebCore/storage/SQLTransactionErrorCallback.h
    21202122

  • trunk/WebCore/WebCore.gypi

    r47165 r47170  
    29632963            'storage/SQLTransaction.h',
    29642964            'storage/SQLTransactionCallback.h',
     2965            'storage/SQLTransactionCoordinator.h',
     2966            'storage/SQLTransactionCoordinator.cpp',
    29652967            'storage/SQLTransactionErrorCallback.h',
    29662968            'storage/Storage.cpp',

  • trunk/WebCore/WebCore.vcproj/WebCore.vcproj

    r47165 r47170  
    3073730737                        </File>
    3073830738                        <File
     30739                                RelativePath="..\storage\SQLTransactionCoordinator.h"
     30740                                >
     30741                        </File>
     30742                        <File
     30743                                RelativePath="..\storage\SQLTransactionCoordinator.cpp"
     30744                                >
     30745                        </File>
     30746                        <File
    3073930747                                RelativePath="..\storage\SQLTransactionErrorCallback.h"
    3074030748                                >

  • trunk/WebCore/WebCore.xcodeproj/project.pbxproj

    r47165 r47170  
    38633863                B5A684220FFABE9800D24689 /* SQLiteFileSystem.h in Headers */ = {isa = PBXBuildFile; fileRef = B5A684210FFABE9800D24689 /* SQLiteFileSystem.h */; };
    38643864                B5A684240FFABEAA00D24689 /* SQLiteFileSystem.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B5A684230FFABEAA00D24689 /* SQLiteFileSystem.cpp */; };
     3865                B5C1123B102B6C4600096578 /* SQLTransactionCoordinator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B5C11239102B6C4600096578 /* SQLTransactionCoordinator.cpp */; };
     3866                B5C1123C102B6C4600096578 /* SQLTransactionCoordinator.h in Headers */ = {isa = PBXBuildFile; fileRef = B5C1123A102B6C4600096578 /* SQLTransactionCoordinator.h */; };
    38653867                BC00F0040E0A185500FD04E3 /* DOMFile.h in Headers */ = {isa = PBXBuildFile; fileRef = BC00EFFE0E0A185500FD04E3 /* DOMFile.h */; };
    38663868                BC00F0050E0A185500FD04E3 /* DOMFile.mm in Sources */ = {isa = PBXBuildFile; fileRef = BC00EFFF0E0A185500FD04E3 /* DOMFile.mm */; };
    … …  
    87558757                B5A684210FFABE9800D24689 /* SQLiteFileSystem.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SQLiteFileSystem.h; path = sql/SQLiteFileSystem.h; sourceTree = "<group>"; };
    87568758                B5A684230FFABEAA00D24689 /* SQLiteFileSystem.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SQLiteFileSystem.cpp; path = sql/SQLiteFileSystem.cpp; sourceTree = "<group>"; };
     8759                B5C11239102B6C4600096578 /* SQLTransactionCoordinator.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SQLTransactionCoordinator.cpp; sourceTree = "<group>"; };
     8760                B5C1123A102B6C4600096578 /* SQLTransactionCoordinator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SQLTransactionCoordinator.h; sourceTree = "<group>"; };
    87578761                BC00EFFE0E0A185500FD04E3 /* DOMFile.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DOMFile.h; sourceTree = "<group>"; };
    87588762                BC00EFFF0E0A185500FD04E3 /* DOMFile.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DOMFile.mm; sourceTree = "<group>"; };
    … …  
    98409844                        isa = PBXGroup;
    98419845                        children = (
     9846                                B5C11239102B6C4600096578 /* SQLTransactionCoordinator.cpp */,
     9847                                B5C1123A102B6C4600096578 /* SQLTransactionCoordinator.h */,
    98429848                                514185ED0CD65F0400763C99 /* ChangeVersionWrapper.cpp */,
    98439849                                514185EC0CD65F0400763C99 /* ChangeVersionWrapper.h */,
    … …  
    1725317259                                49484FCE102CF23C00187DD3 /* CanvasStyle.h in Headers */,
    1725417260                                416E29A6102FA962007FC14E /* WorkerReportingProxy.h in Headers */,
     17261                                B5C1123C102B6C4600096578 /* SQLTransactionCoordinator.h in Headers */,
    1725517262                        );
    1725617263                        runOnlyForDeploymentPostprocessing = 0;
    … …  
    1931119318                                9392262D1032107B006E7D5D /* JSHTMLCanvasElementCustom.cpp in Sources */,
    1931219319                                9392262F10321084006E7D5D /* JSCSSRuleListCustom.cpp in Sources */,
     19320                                B5C1123B102B6C4600096578 /* SQLTransactionCoordinator.cpp in Sources */,
    1931319321                        );
    1931419322                        runOnlyForDeploymentPostprocessing = 0;

  • trunk/WebCore/storage/Database.cpp

    r45594 r47170  
    5151#include "SQLiteStatement.h"
    5252#include "SQLResultSet.h"
     53#include "SQLTransactionCoordinator.h"
    5354#include <wtf/MainThread.h>
    5455#endif
    … …  
    540541}
    541542
    542 void Database::scheduleTransactionStep(SQLTransaction* transaction)
    543 {
    544     if (m_document->databaseThread()) {
    545         RefPtr<DatabaseTransactionTask> task = DatabaseTransactionTask::create(transaction);
    546         LOG(StorageAPI, "Scheduling DatabaseTransactionTask %p for the transaction step\n", task.get());
     543void Database::scheduleTransactionStep(SQLTransaction* transaction, bool immediately)
     544{
     545    if (!m_document->databaseThread())
     546        return;
     547
     548    RefPtr<DatabaseTransactionTask> task = DatabaseTransactionTask::create(transaction);
     549    LOG(StorageAPI, "Scheduling DatabaseTransactionTask %p for the transaction step\n", task.get());
     550    if (immediately)
     551        m_document->databaseThread()->scheduleImmediateTask(task.release());
     552    else
    547553        m_document->databaseThread()->scheduleTask(task.release());
    548     }
    549554}
    550555
    … …  
    584589}
    585590
     591SQLTransactionCoordinator* Database::transactionCoordinator() const
     592{
     593    return m_document->databaseThread()->transactionCoordinator();
     594}
     595
    586596String Database::version() const
    587597{

  • trunk/WebCore/storage/Database.h

    r45594 r47170  
    5858class SQLResultSet;
    5959class SQLTransactionCallback;
     60class SQLTransactionCoordinator;
    6061class SQLTransactionErrorCallback;
    6162class SQLValue;
    … …  
    117118    Vector<String> performGetTableNames();
    118119
     120    SQLTransactionCoordinator* transactionCoordinator() const;
     121
    119122private:
    120123    Database(Document* document, const String& name, const String& expectedVersion);
    … …  
    124127    void scheduleTransaction();
    125128    void scheduleTransactionCallback(SQLTransaction*);
    126     void scheduleTransactionStep(SQLTransaction* transaction);
     129    void scheduleTransactionStep(SQLTransaction* transaction, bool immediately = false);
    127130   
    128131    MessageQueue<RefPtr<SQLTransaction> > m_transactionQueue;

  • trunk/WebCore/storage/DatabaseThread.cpp

    r45594 r47170  
    3636#include "DatabaseTask.h"
    3737#include "Logging.h"
     38#include "SQLTransactionCoordinator.h"
    3839
    3940namespace WebCore {
    … …  
    4142DatabaseThread::DatabaseThread()
    4243    : m_threadID(0)
     44    , m_transactionCoordinator(new SQLTransactionCoordinator())
    4345{
    4446    m_selfRef = this;
    … …  
    9799        pool.cycle();
    98100    }
     101
     102    // Clean up the list of all pending transactions on this database thread
     103    m_transactionCoordinator->shutdown();
    99104
    100105    LOG(StorageAPI, "About to detach thread %i and clear the ref to DatabaseThread %p, which currently has %i ref(s)", m_threadID, this, refCount());

  • trunk/WebCore/storage/DatabaseThread.h

    r45594 r47170  
    3434#include <wtf/HashSet.h>
    3535#include <wtf/MessageQueue.h>
     36#include <wtf/OwnPtr.h>
    3637#include <wtf/PassRefPtr.h>
    3738#include <wtf/RefPtr.h>
    … …  
    4344class DatabaseTask;
    4445class Document;
     46class SQLTransactionCoordinator;
    4547
    4648class DatabaseThread : public ThreadSafeShared<DatabaseThread> {
    … …  
    6163    ThreadIdentifier getThreadID() { return m_threadID; }
    6264
     65    SQLTransactionCoordinator* transactionCoordinator() { return m_transactionCoordinator.get(); }
     66
    6367private:
    6468    DatabaseThread();
    … …  
    7680    typedef HashSet<RefPtr<Database> > DatabaseSet;
    7781    DatabaseSet m_openDatabaseSet;
     82
     83    OwnPtr<SQLTransactionCoordinator> m_transactionCoordinator;
    7884};
    7985

  • trunk/WebCore/storage/SQLTransaction.cpp

    r44274 r47170  
    5151#include "SQLStatementCallback.h"
    5252#include "SQLStatementErrorCallback.h"
     53#include "SQLTransactionCoordinator.h"
    5354#include "SQLValue.h"
    5455
    … …  
    6869SQLTransaction::SQLTransaction(Database* db, PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback, PassRefPtr<VoidCallback> successCallback,
    6970                               PassRefPtr<SQLTransactionWrapper> wrapper)
    70     : m_nextStep(&SQLTransaction::openTransactionAndPreflight)
     71    : m_nextStep(&SQLTransaction::acquireLock)
    7172    , m_executeSqlAllowed(false)
    7273    , m_database(db)
    … …  
    7778    , m_shouldRetryCurrentStatement(false)
    7879    , m_modifiedDatabase(false)
     80    , m_lockAcquired(false)
    7981{
    8082    ASSERT(m_database);
    … …  
    117119const char* SQLTransaction::debugStepName(SQLTransaction::TransactionStepMethod step)
    118120{
    119     if (step == &SQLTransaction::openTransactionAndPreflight)
     121    if (step == &SQLTransaction::acquireLock)
     122        return "acquireLock";
     123    else if (step == &SQLTransaction::openTransactionAndPreflight)
    120124        return "openTransactionAndPreflight";
    121125    else if (step == &SQLTransaction::runStatements)
    … …  
    158162        m_sqliteTransaction.clear();
    159163    }
     164
     165    if (m_lockAcquired)
     166        m_database->transactionCoordinator()->releaseLock(this);
    160167}
    161168
    … …  
    165172    LOG(StorageAPI, "Step %s\n", debugStepName(m_nextStep));
    166173
    167     ASSERT(m_nextStep == &SQLTransaction::openTransactionAndPreflight ||
     174    ASSERT(m_nextStep == &SQLTransaction::acquireLock ||
     175           m_nextStep == &SQLTransaction::openTransactionAndPreflight ||
    168176           m_nextStep == &SQLTransaction::runStatements ||
    169177           m_nextStep == &SQLTransaction::postflightAndCommit ||
    … …  
    196204}
    197205
     206void SQLTransaction::acquireLock()
     207{
     208    m_database->transactionCoordinator()->acquireLock(this);
     209}
     210
     211void SQLTransaction::lockAcquired()
     212{
     213    m_lockAcquired = true;
     214    m_nextStep = &SQLTransaction::openTransactionAndPreflight;
     215    LOG(StorageAPI, "Scheduling openTransactionAndPreflight immediately for transaction %p\n", this);
     216    m_database->scheduleTransactionStep(this, true);
     217}
     218
    198219void SQLTransaction::openTransactionAndPreflight()
    199220{
    200221    ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
     222    ASSERT(m_lockAcquired);
    201223
    202224    LOG(StorageAPI, "Opening and preflighting transaction %p", this);
    … …  
    274296void SQLTransaction::runStatements()
    275297{
     298    ASSERT(m_lockAcquired);
     299
    276300    // If there is a series of statements queued up that are all successful and have no associated
    277301    // SQLStatementCallback objects, then we can burn through the queue
    … …  
    412436void SQLTransaction::postflightAndCommit()
    413437{   
     438    ASSERT(m_lockAcquired);
     439
    414440    // Transaction Step 7 - Peform postflight steps, jumping to the error callback if they fail
    415441    if (m_wrapper && !m_wrapper->performPostflight(this)) {
    … …  
    470496void SQLTransaction::cleanupAfterSuccessCallback()
    471497{
     498    ASSERT(m_lockAcquired);
     499
    472500    // Transaction Step 11 - End transaction steps
    473501    // There is no next step
    … …  
    475503    ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
    476504    m_nextStep = 0;
     505
     506    // Release the lock on this database
     507    m_database->transactionCoordinator()->releaseLock(this);
    477508}
    478509
    … …  
    517548void SQLTransaction::cleanupAfterTransactionErrorCallback()
    518549{
     550    ASSERT(m_lockAcquired);
     551
    519552    m_database->m_databaseAuthorizer->disable();
    520553    if (m_sqliteTransaction) {
    … …  
    541574    m_callback = 0;
    542575    m_errorCallback = 0;
     576
     577    // Now release the lock on this database
     578    m_database->transactionCoordinator()->releaseLock(this);
    543579}
    544580

  • trunk/WebCore/storage/SQLTransaction.h

    r44155 r47170  
    7474                    PassRefPtr<SQLStatementCallback> callback, PassRefPtr<SQLStatementErrorCallback> callbackError, ExceptionCode& e);
    7575                                       
     76    void lockAcquired();
    7677    bool performNextStep();
    7778    void performPendingCallback();
    … …  
    8990    void checkAndHandleClosedDatabase();
    9091   
     92    void acquireLock();
    9193    void openTransactionAndPreflight();
    9294    void deliverTransactionCallback();
    … …  
    121123    bool m_shouldRetryCurrentStatement;
    122124    bool m_modifiedDatabase;
     125    bool m_lockAcquired;
    123126   
    124127    Mutex m_statementMutex;
Note: See TracChangeset for help on using the changeset viewer.