Bug 202705 - IndexedDB: on-going IDBTransaction is not active any more after process is suspended
Summary: IndexedDB: on-going IDBTransaction is not active any more after process is su...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 12
Hardware: iPhone / iPad iOS 13
: P2 Major
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-08 14:04 PDT by mrschmidt
Modified: 2020-03-23 10:10 PDT (History)
10 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description mrschmidt 2019-10-08 14:04:21 PDT
We work on the Firestore SDK, which provide real-time data synchronization for users of Google Cloud Firestore. As part of our offering, we ship an IndexedDb-enabled Web client. Our users have reported a severe problem with iOS 13 that essentially means that we are currently unable to support iOS 13. 

With iOS 13, we have observed the following behavior:

1) The Firestore SDK opens a new IndexedDb transaction/
2) The Safari tab goes in the background.
3) WebKit terminates the network, which stops the IDBServer (https://trac.webkit.org/browser/trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp#L2117)
4) The app goes back in the foreground and resumes its operation. Firestore tries to use the transaction it opened in step 1 and receives "Attempt to get a record from database without an in-progress transaction"

We believe that this is likely a regression introduced by the fix for https://bugs.webkit.org/show_bug.cgi?id=197050

See https://github.com/firebase/firebase-js-sdk/issues/2232 for further comments and analysis.
Comment 1 Sihui Liu 2019-10-08 16:26:02 PDT
(In reply to mrschmidt from comment #0)
> We work on the Firestore SDK, which provide real-time data synchronization
> for users of Google Cloud Firestore. As part of our offering, we ship an
> IndexedDb-enabled Web client. Our users have reported a severe problem with
> iOS 13 that essentially means that we are currently unable to support iOS
> 13. 
> 
> With iOS 13, we have observed the following behavior:
> 
> 1) The Firestore SDK opens a new IndexedDb transaction/
> 2) The Safari tab goes in the background.
> 3) WebKit terminates the network, which stops the IDBServer
> (https://trac.webkit.org/browser/trunk/Source/WebKit/NetworkProcess/
> NetworkProcess.cpp#L2117)
> 4) The app goes back in the foreground and resumes its operation. Firestore
> tries to use the transaction it opened in step 1 and receives "Attempt to
> get a record from database without an in-progress transaction"
> 
> We believe that this is likely a regression introduced by the fix for
> https://bugs.webkit.org/show_bug.cgi?id=197050
> 
> See https://github.com/firebase/firebase-js-sdk/issues/2232 for further
> comments and analysis.

Hi,

Thanks for the report. That change was made to protect WebKit process from being killed when holding database lock for ongoing transactions in the background. Process getting killed could error out all IDB connections and lead to some bad results.

The current WebKit behavior is when Webkit process is suspended (which should happen some time after app is backgrounded), ongoing IDB transactions are forcibly aborted.

We may figure out something smarter than this for the backgrounding problem. For temporary workaround on iOS 13, you could start a new transaction on the same db and replay IDB requests if needed (may need some effort to record the requests in an on-going transaction).
Comment 2 mrschmidt 2019-10-08 16:36:02 PDT
Sihui, thank you for replying so quickly. The strategy you suggested is what we are going to implement as a mitigation, but it requires us to audit and potentially rewrite all of our usages of IndexedDb to ensure that the code that runs in our transactions is idempotent. This is a significant engineering challenge for us and makes IndexedDb development for everyone much more difficult. 

Would it be possible to delay suspension of the process until all IndexedDb transactions are committed?
Comment 3 Sihui Liu 2019-10-08 18:50:46 PDT
(In reply to mrschmidt from comment #2)
> Sihui, thank you for replying so quickly. The strategy you suggested is what
> we are going to implement as a mitigation, but it requires us to audit and
> potentially rewrite all of our usages of IndexedDb to ensure that the code
> that runs in our transactions is idempotent. This is a significant
> engineering challenge for us and makes IndexedDb development for everyone
> much more difficult. 
> 
> Would it be possible to delay suspension of the process until all IndexedDb
> transactions are committed?

We already request delay if we know there are on-going IDB transactions, but there is a timeout for that. (And currently the web page would not know when the clock is ticking.) After the timeout, WebKit process will get a final notification from system, and have to do cleanup immediately in case processes get suspended in weird state or get killed. This cleanup includes aborting on-going transactions (because transaction should not be interrupted and after process is suspended, other active process may perform on the same storage), so you can view this transaction error as a result of process suspension on iOS.

To help mitigate this issue, probably you could try committing more often, or making transaction shorter, if possible.
Comment 4 Mauricio W 2019-10-09 09:56:17 PDT
I have two related questions:

1) Would it be possible to pause an in-progress transaction instead of aborting?

2) In the tests for the patch for 196372 (https://bugs.webkit.org/show_bug.cgi?id=197050), it is implied that the transaction is resumed after being terminated. Is this actually the expected functionality, or am I reading this wrong?
See: https://trac.webkit.org/browser/trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBSuspendImminently.html#L27
https://trac.webkit.org/browser/trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/IndexedDBSuspendImminently.mm#L94)
Comment 5 Sihui Liu 2019-10-09 10:32:41 PDT
(In reply to Mauricio W from comment #4)
> I have two related questions:
> 
> 1) Would it be possible to pause an in-progress transaction instead of
> aborting?
> 
No if the process is put suspended.

> 2) In the tests for the patch for 196372
> (https://bugs.webkit.org/show_bug.cgi?id=197050), it is implied that the
> transaction is resumed after being terminated. Is this actually the expected
> functionality, or am I reading this wrong?
> See:
> https://trac.webkit.org/browser/trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/
> IndexedDBSuspendImminently.html#L27
> https://trac.webkit.org/browser/trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/
> IndexedDBSuspendImminently.mm#L94)

The test starts 10 transactions, and currently transactions are executed serially on the same database. When process is suspended, we only abort the transaction in progress, and stop starting new transaction. When process is resumed, we start processing new transactions.

That also implies if you have multiple transactions, and if you have never received from callback from requests in some transaction, you don't need to worry about that transaction being aborted because of process suspension.
Comment 6 Maciej Stachowiak 2020-03-23 10:09:49 PDT
If a transaction is aborted, could we save it and replay it automatically?
Comment 7 Radar WebKit Bug Importer 2020-03-23 10:10:28 PDT
<rdar://problem/60779616>