Changeset 200598 in webkit


Ignore:
Timestamp:
May 9, 2016 4:59:55 PM (8 years ago)
Author:
beidson@apple.com
Message:

Modern IDB: Prevent the same transaction from being commited/aborted twice.
​https://bugs.webkit.org/show_bug.cgi?id=157494

Reviewed by Tim Horton.

When a page navigates or a worker terminates, in rare cases with specific unfortunate timing, the IDBServer
might receive a request to commit/abort a transaction that is already in the process of committing/aborting.

By moving transactions that are finishing into their own map we can at least detect this situation and
return an error. This seems like an improvement over some mysterious ASSERTs/timeouts.

No new tests:
While apparent that this is at least partially to blame for some existing timeouts/ASSERTs, I could not nail
down a reliable way to reproduce this with a dedicated test.

  • Modules/indexeddb/server/UniqueIDBDatabase.cpp:

(WebCore::IDBServer::UniqueIDBDatabase::~UniqueIDBDatabase):
(WebCore::IDBServer::UniqueIDBDatabase::performCurrentDeleteOperation):
(WebCore::IDBServer::UniqueIDBDatabase::didDeleteBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::prepareToFinishTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::commitTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::didPerformCommitTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::abortTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::didPerformAbortTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::hasUnfinishedTransactions):
(WebCore::IDBServer::UniqueIDBDatabase::operationAndTransactionTimerFired):
(WebCore::IDBServer::UniqueIDBDatabase::takeNextRunnableTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::transactionCompleted): Renamed from inProgressTransactionCompleted.
(WebCore::IDBServer::UniqueIDBDatabase::inProgressTransactionCompleted): Deleted.

  • Modules/indexeddb/server/UniqueIDBDatabase.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r200596 r200598  
     12016-05-09  Brady Eidson  <beidson@apple.com>
     2
     3        Modern IDB: Prevent the same transaction from being commited/aborted twice.
     4        https://bugs.webkit.org/show_bug.cgi?id=157494
     5
     6        Reviewed by Tim Horton.
     7
     8        When a page navigates or a worker terminates, in rare cases with specific unfortunate timing, the IDBServer
     9        might receive a request to commit/abort a transaction that is already in the process of committing/aborting.
     10       
     11        By moving transactions that are finishing into their own map we can at least detect this situation and
     12        return an error. This seems like an improvement over some mysterious ASSERTs/timeouts.
     13
     14        No new tests:
     15        While apparent that this is at least partially to blame for some existing timeouts/ASSERTs, I could not nail
     16        down a reliable way to reproduce this with a dedicated test.
     17       
     18        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
     19        (WebCore::IDBServer::UniqueIDBDatabase::~UniqueIDBDatabase):
     20        (WebCore::IDBServer::UniqueIDBDatabase::performCurrentDeleteOperation):
     21        (WebCore::IDBServer::UniqueIDBDatabase::didDeleteBackingStore):
     22        (WebCore::IDBServer::UniqueIDBDatabase::prepareToFinishTransaction):
     23        (WebCore::IDBServer::UniqueIDBDatabase::commitTransaction):
     24        (WebCore::IDBServer::UniqueIDBDatabase::didPerformCommitTransaction):
     25        (WebCore::IDBServer::UniqueIDBDatabase::abortTransaction):
     26        (WebCore::IDBServer::UniqueIDBDatabase::didPerformAbortTransaction):
     27        (WebCore::IDBServer::UniqueIDBDatabase::hasUnfinishedTransactions):
     28        (WebCore::IDBServer::UniqueIDBDatabase::operationAndTransactionTimerFired):
     29        (WebCore::IDBServer::UniqueIDBDatabase::takeNextRunnableTransaction):
     30        (WebCore::IDBServer::UniqueIDBDatabase::transactionCompleted): Renamed from inProgressTransactionCompleted.
     31        (WebCore::IDBServer::UniqueIDBDatabase::inProgressTransactionCompleted): Deleted.
     32        * Modules/indexeddb/server/UniqueIDBDatabase.h:
     33
    1342016-05-09  Tim Horton  <timothy_horton@apple.com>
    235

  • trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp

    r199668 r200598  
    6161    LOG(IndexedDB, "UniqueIDBDatabase::~UniqueIDBDatabase() (%p) %s", this, m_identifier.debugString().utf8().data());
    6262    ASSERT(!hasAnyPendingCallbacks());
    63     ASSERT(m_inProgressTransactions.isEmpty());
     63    ASSERT(!hasUnfinishedTransactions());
    6464    ASSERT(m_pendingTransactions.isEmpty());
    6565    ASSERT(m_openDatabaseConnections.isEmpty());
    … …  
    195195    }
    196196
    197     if (!m_inProgressTransactions.isEmpty())
     197    if (hasUnfinishedTransactions())
    198198        return;
    199199
    … …  
    251251    ASSERT(m_currentOpenDBRequest->isDeleteRequest());
    252252    ASSERT(!hasAnyPendingCallbacks());
    253     ASSERT(m_inProgressTransactions.isEmpty());
     253    ASSERT(!hasUnfinishedTransactions());
    254254    ASSERT(m_pendingTransactions.isEmpty());
    255255    ASSERT(m_openDatabaseConnections.isEmpty());
    … …  
    984984}
    985985
     986bool UniqueIDBDatabase::prepareToFinishTransaction(UniqueIDBDatabaseTransaction& transaction)
     987{
     988    auto takenTransaction = m_inProgressTransactions.take(transaction.info().identifier());
     989    if (!takenTransaction)
     990        return false;
     991
     992    ASSERT(!m_finishingTransactions.contains(transaction.info().identifier()));
     993    m_finishingTransactions.set(transaction.info().identifier(), WTFMove(takenTransaction));
     994
     995    return true;
     996}
     997
    986998void UniqueIDBDatabase::commitTransaction(UniqueIDBDatabaseTransaction& transaction, ErrorCallback callback)
    987999{
    … …  
    9911003    ASSERT(&transaction.databaseConnection().database() == this);
    9921004
    993     if (m_versionChangeTransaction == &transaction) {
    994         ASSERT(!m_versionChangeDatabaseConnection || &m_versionChangeTransaction->databaseConnection() == m_versionChangeDatabaseConnection);
    995         ASSERT(m_databaseInfo->version() == transaction.info().newVersion());
    996 
    997         invokeOperationAndTransactionTimer();
    998     }
    999 
    10001005    uint64_t callbackID = storeCallback(callback);
     1006
     1007    if (!prepareToFinishTransaction(transaction)) {
     1008        performErrorCallback(callbackID, { IDBDatabaseException::UnknownError, ASCIILiteral("Attempt to commit transaction that is already finishing") });
     1009        return;
     1010    }
     1011
    10011012    m_server.postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performCommitTransaction, callbackID, transaction.info().identifier()));
    10021013}
    … …  
    10181029    performErrorCallback(callbackIdentifier, error);
    10191030
    1020     inProgressTransactionCompleted(transactionIdentifier);
     1031    transactionCompleted(m_finishingTransactions.take(transactionIdentifier));
    10211032}
    10221033
    … …  
    10291040
    10301041    uint64_t callbackID = storeCallback(callback);
     1042
     1043    if (!prepareToFinishTransaction(transaction)) {
     1044        performErrorCallback(callbackID, { IDBDatabaseException::UnknownError, ASCIILiteral("Attempt to abort transaction that is already finishing") });
     1045        return;
     1046    }
     1047
    10311048    m_server.postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performAbortTransaction, callbackID, transaction.info().identifier()));
    10321049}
    … …  
    10601077    LOG(IndexedDB, "(main) UniqueIDBDatabase::didPerformAbortTransaction");
    10611078
     1079    auto transaction = m_finishingTransactions.take(transactionIdentifier);
     1080    ASSERT(transaction);
     1081
    10621082    if (m_versionChangeTransaction && m_versionChangeTransaction->info().identifier() == transactionIdentifier) {
     1083        ASSERT(m_versionChangeTransaction == transaction);
    10631084        ASSERT(!m_versionChangeDatabaseConnection || &m_versionChangeTransaction->databaseConnection() == m_versionChangeDatabaseConnection);
    10641085        ASSERT(m_versionChangeTransaction->originalDatabaseInfo());
    … …  
    10681089    performErrorCallback(callbackIdentifier, error);
    10691090
    1070     inProgressTransactionCompleted(transactionIdentifier);
     1091    transactionCompleted(WTFMove(transaction));
    10711092}
    10721093
    … …  
    11281149}
    11291150
     1151bool UniqueIDBDatabase::hasUnfinishedTransactions() const
     1152{
     1153    return !m_inProgressTransactions.isEmpty() || !m_finishingTransactions.isEmpty();
     1154}
     1155
    11301156void UniqueIDBDatabase::invokeOperationAndTransactionTimer()
    11311157{
    … …  
    11451171    if (!m_backingStoreIsEphemeral && !isCurrentlyInUse()) {
    11461172        ASSERT(m_pendingTransactions.isEmpty());
    1147         ASSERT(m_inProgressTransactions.isEmpty());
     1173        ASSERT(!hasUnfinishedTransactions());
    11481174        m_server.closeUniqueIDBDatabase(*this);
    11491175        return;
    … …  
    12241250{
    12251251    hadDeferredTransactions = false;
    1226     if (!m_backingStoreSupportsSimultaneousTransactions && !m_inProgressTransactions.isEmpty()) {
     1252    if (!m_backingStoreSupportsSimultaneousTransactions && hasUnfinishedTransactions()) {
    12271253        LOG(IndexedDB, "UniqueIDBDatabase::takeNextRunnableTransaction - Backing store only supports 1 transaction, and we already have 1");
    12281254        return nullptr;
    … …  
    12801306}
    12811307
    1282 void UniqueIDBDatabase::inProgressTransactionCompleted(const IDBResourceIdentifier& transactionIdentifier)
    1283 {
    1284     auto transaction = m_inProgressTransactions.take(transactionIdentifier);
     1308void UniqueIDBDatabase::transactionCompleted(RefPtr<UniqueIDBDatabaseTransaction>&& transaction)
     1309{
    12851310    ASSERT(transaction);
     1311    ASSERT(!m_inProgressTransactions.contains(transaction->info().identifier()));
     1312    ASSERT(!m_finishingTransactions.contains(transaction->info().identifier()));
    12861313
    12871314    for (auto objectStore : transaction->objectStoreIdentifiers()) {

  • trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h

    r199668 r200598  
    125125
    126126    void activateTransactionInBackingStore(UniqueIDBDatabaseTransaction&);
    127     void inProgressTransactionCompleted(const IDBResourceIdentifier&);
     127    void transactionCompleted(RefPtr<UniqueIDBDatabaseTransaction>&&);
    128128
    129129    // Database thread operations
    … …  
    177177    bool hasAnyPendingCallbacks() const;
    178178    bool isCurrentlyInUse() const;
     179    bool hasUnfinishedTransactions() const;
    179180
    180181    void invokeOperationAndTransactionTimer();
    181182    void operationAndTransactionTimerFired();
    182183    RefPtr<UniqueIDBDatabaseTransaction> takeNextRunnableTransaction(bool& hadDeferredTransactions);
     184
     185    bool prepareToFinishTransaction(UniqueIDBDatabaseTransaction&);
    183186
    184187    IDBServer& m_server;
    … …  
    212215    Deque<RefPtr<UniqueIDBDatabaseTransaction>> m_pendingTransactions;
    213216    HashMap<IDBResourceIdentifier, RefPtr<UniqueIDBDatabaseTransaction>> m_inProgressTransactions;
     217    HashMap<IDBResourceIdentifier, RefPtr<UniqueIDBDatabaseTransaction>> m_finishingTransactions;
    214218
    215219    // The keys into these sets are the object store ID.
Note: See TracChangeset for help on using the changeset viewer.