Skip to content
Permalink
Browse files
Crash in com.apple.WebCore: WebCore::IDBTransaction::pendingOperation…
…TimerFired + 72

https://bugs.webkit.org/show_bug.cgi?id=195214
<rdar://problem/48461116>

Reviewed by Geoffrey Garen.

When IDBTransaction is ready to commit, a commit operation would be schedule to
m_pendingTransactionOperationQueue. If connection to IDBServer is lost, pending operations are moved to
m_transactionOperationsInProgressQueue and will be completed with TransactionOperation::doComplete. doComplete
executes complete function of the operation, clears the complete function, and then removes the operation from
m_transactionOperationsInProgressQueue. In doComplete, we do early return when complete function is null,
since the doComplete could be invoked twice due to the race conditions between receiving "operation complete"
message from server and client-side abort.

However, commit operation does not have a complete function because it should be the last operation of
transaction and it gets removed from queue in its perform function. A commit operation would not be removed from
m_transactionOperationsInProgressQueue because of the early return. It would be removed from
m_transactionOperationMap, which may hold the last ref to the commit operation, in
IDBTransaction::connectionClosedFromServer. In this case, when pendingOperationTimerFired is called later, the
commit operation left in m_transactionOperationsInProgressQueue would be used and found to be freed. We should
not use null check of complete function to decide whether an operation is completed.

* Modules/indexeddb/client/TransactionOperation.h:
(WebCore::IDBClient::TransactionOperation::doComplete):


Canonical link: https://commits.webkit.org/209747@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242608 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
szewai committed Mar 7, 2019
1 parent d55bf15 commit 7a810d26d7cf1f0957dafb314c2d8514cd690cc4
Showing 2 changed files with 35 additions and 7 deletions.
@@ -1,3 +1,30 @@
2019-03-07 Sihui Liu <sihui_liu@apple.com>

Crash in com.apple.WebCore: WebCore::IDBTransaction::pendingOperationTimerFired + 72
https://bugs.webkit.org/show_bug.cgi?id=195214
<rdar://problem/48461116>

Reviewed by Geoffrey Garen.

When IDBTransaction is ready to commit, a commit operation would be schedule to
m_pendingTransactionOperationQueue. If connection to IDBServer is lost, pending operations are moved to
m_transactionOperationsInProgressQueue and will be completed with TransactionOperation::doComplete. doComplete
executes complete function of the operation, clears the complete function, and then removes the operation from
m_transactionOperationsInProgressQueue. In doComplete, we do early return when complete function is null,
since the doComplete could be invoked twice due to the race conditions between receiving "operation complete"
message from server and client-side abort.

However, commit operation does not have a complete function because it should be the last operation of
transaction and it gets removed from queue in its perform function. A commit operation would not be removed from
m_transactionOperationsInProgressQueue because of the early return. It would be removed from
m_transactionOperationMap, which may hold the last ref to the commit operation, in
IDBTransaction::connectionClosedFromServer. In this case, when pendingOperationTimerFired is called later, the
commit operation left in m_transactionOperationsInProgressQueue would be used and found to be freed. We should
not use null check of complete function to decide whether an operation is completed.

* Modules/indexeddb/client/TransactionOperation.h:
(WebCore::IDBClient::TransactionOperation::doComplete):

2019-03-07 John Wilander <wilander@apple.com>

Resource Load Statistics: Log first-party navigations with link decoration
@@ -91,16 +91,16 @@ class TransactionOperation : public ThreadSafeRefCounted<TransactionOperation> {
// Due to race conditions between the server sending an "operation complete" message and the client
// forcefully aborting an operation, it's unavoidable that this method might be called twice.
// It's okay to handle that gracefully with an early return.
if (!m_completeFunction)
if (m_didComplete)
return;
m_didComplete = true;

m_completeFunction(data);
if (m_completeFunction) {
m_completeFunction(data);
// m_completeFunction should not hold ref to this TransactionOperation after its execution.
m_completeFunction = { };
}
m_transaction->operationCompletedOnClient(*this);

// m_completeFunction might be holding the last ref to this TransactionOperation,
// so we need to do this trick to null it out without first destroying it.
Function<void(const IDBResultData&)> oldCompleteFunction;
std::swap(m_completeFunction, oldCompleteFunction);
}

const IDBResourceIdentifier& identifier() const { return m_identifier; }
@@ -141,6 +141,7 @@ class TransactionOperation : public ThreadSafeRefCounted<TransactionOperation> {
Ref<Thread> m_originThread { Thread::current() };
RefPtr<IDBRequest> m_idbRequest;
bool m_nextRequestCanGoToServer { true };
bool m_didComplete { false };
};

class TransactionOperationImpl final : public TransactionOperation {

0 comments on commit 7a810d2

Please sign in to comment.