[webkit-changes] [WebKit/WebKit] a2fdb8: Crash under UniqueIDBDatabaseTransaction::commit

[Sihui]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: a2fdb85c4c4a5bfba20550813bc8dfb7b7691b3a
      https://github.com/WebKit/WebKit/commit/a2fdb85c4c4a5bfba20550813bc8dfb7b7691b3a
  Author: Sihui Liu <sihui_liu at apple.com>
  Date:   2024-03-21 (Thu, 21 Mar 2024)

  Changed paths:
    M Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
    M Source/WebCore/Modules/indexeddb/IDBTransaction.h
    M Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp
    M Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h
    M Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp
    M Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h
    M Source/WebCore/Modules/indexeddb/client/IDBConnectionToServerDelegate.h
    M Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
    M Source/WebCore/Modules/indexeddb/server/IDBServer.h
    M Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
    M Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h
    M Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.cpp
    M Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h
    M Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp
    M Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h
    M Source/WebKit/NetworkProcess/storage/NetworkStorageManager.messages.in
    M Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp
    M Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.h

  Log Message:
  -----------
  Crash under UniqueIDBDatabaseTransaction::commit
https://bugs.webkit.org/show_bug.cgi?id=271300
rdar://114552467

Reviewed by Per Arne Vollan and Chris Dumez.

>From crash reports, we know UniqueIDBDatabaseTransaction tries to take item from m_requestResults while it is empty,
which means pendingRequestCount is bigger than the size of m_requestResults. In current implementation, the parameter
pendingRequestCount indicates the number of request that client has not received result when submitting transaction
commit. UniqueIDBDatabaseTransaction will search whether there is an error in the last pendingRequestCount results
before submitting the commit operation to database. If there is no error, it asks database to commit transaction; if
there is an error, it asks database to abort the transaction as the error is not handled by client.

Chris pointed out an issue that m_requestResults is updated after async quota check in UniqueIDBDatabase, while
searching error in m_requestResults happens before the async quota check, which can be a direct cause to this issue.
To fix that, this patch moves the check (UniqueIDBDatabaseTransaction::shouldAbortDueToUnhandledRequestError) to inside
UniqueIDBDatabase::commitTransaction and after quota check. This guarantees the search happens after all requests
complete.

There are two other problems. One is that client is using the number of unfinished requests as pendingRequestCount and
server is recording all request results in m_requestResults, but one request may generate multiple results (e.g. a
cursor request could generate one result for each advancing operation). The patch fixes this by making client track
count of handled request results (IDBTransaction::m_handledRequestResultsCount) and pass thatto server. Another is that
certain operations (like createObjectStore) are not associated with request, so their result should not be put in
m_requestResults. This patch fixes that by removing the calls in UniqueIDBDatabaseTransaction.

* Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:
(WebCore::IDBTransaction::finishedDispatchEventForRequest):
(WebCore::IDBTransaction::commitInternal):
(WebCore::IDBTransaction::commitOnServer):
* Source/WebCore/Modules/indexeddb/IDBTransaction.h:
* Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp:
(WebCore::IDBClient::IDBConnectionProxy::commitTransaction):
* Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h:
* Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:
(WebCore::IDBClient::IDBConnectionToServer::commitTransaction):
* Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h:
* Source/WebCore/Modules/indexeddb/client/IDBConnectionToServerDelegate.h:
* Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:
(WebCore::IDBServer::IDBServer::commitTransaction):
* Source/WebCore/Modules/indexeddb/server/IDBServer.h:
* Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::commitTransaction):
* Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h:
* Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.cpp:
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::shouldAbortDueToUnhandledRequestError const):
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::commit):
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::createObjectStore):
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::renameObjectStore):
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::createIndex):
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::deleteIndex):
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::renameIndex):
* Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h:
* Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp:
(WebKit::NetworkStorageManager::commitTransaction):
* Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h:
* Source/WebKit/NetworkProcess/storage/NetworkStorageManager.messages.in:
* Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.cpp:
(WebKit::WebIDBConnectionToServer::commitTransaction):
* Source/WebKit/WebProcess/Databases/IndexedDB/WebIDBConnectionToServer.h:

Canonical link: https://commits.webkit.org/276489@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications