1184410 - It's possible to run CommitOp without ever having really run StartTransactionOp

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Because StartTransactionOp is a TransactionDatabaseOperationBase, while CommitOp is only a DatabaseOperationBase, if the database is invalidated before StartTransactionOp executes on the connection thread it will not execute DoDatabaseWork, and then when CommitOp runs if we had a write transaction it will explode because we never entered the write transaction.

This is responsible for the various !mInReadTransaction assertions.

Rather than reworking one of them to inherit from the same base class it seems easier just to track the transaction started state on the connection thread too, and not execute the CommitOp if we didn't start on the connection thread.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

I'm fairly confident that it is impossible to test this reliably.

Comment 2

[Jan Varga [:janv]]

Comment on attachment 8634517 [details] [diff] [review]
Patch

Review of attachment 8634517 [details] [diff] [review]:
-----------------------------------------------------------------

Ok

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e57d7f2d2c0

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7e57d7f2d2c0

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8634517 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: IndexedDB implementation
[User impact if declined]: Intermittent failures of tests or of web pages if tabs are closed or navigated at exactly the wrong time.
[Describe test coverage new/current, TreeHerder]: Covered by automated tests.
[Risks and why]: Low risk
[String/UUID change made/needed]: N/A

Comment 6

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8634517 [details] [diff] [review]
Patch

Has been in m-c for a few days without any known negative impact. This patch also has coverage from automated tests so should be safe to uplift to Aurora.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7c40cc8b82c3