1013221 - indexedDB stalling forever after calling IDBDatabase.transaction() with duplicate store names

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[David Fahlander]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/34.0.1847.132 Safari/537.36 OPR/21.0.1432.67

Steps to reproduce:

// Let db be an instance of IDBDatabase having an objectStore named "users".
db.transaction(["users", "users"], 'readwrite');





Actual results:

After specifying duplicate store names to the storeNames argument of transaction(), any request to the indexedDB will stall forever, until closing down firefox and opening it up again.



Expected results:

The entire indexedDB should not be stalled by such code. An error could have been thrown or the duplicate argument could have been ignored but.

Comment 1

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

The spec sayeth: "If storeNames is of type sequence<DOMString> then let scope be the set of unique strings in the sequence."

Unforunately we don't perform the uniquification.

Comment 2

[Jon Morton [:jmorton]]

Attachment: 0001-Bug-1013221-Remove-duplicate-object-store-names-when.patch

Comment 3

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

Comment on attachment 8430423 [details] [diff] [review]
0001-Bug-1013221-Remove-duplicate-object-store-names-when.patch

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

r=me with that change

::: dom/indexedDB/IDBTransaction.cpp
@@ +117,5 @@
>  
> +  // Remove any duplicate object store names
> +  uint32_t count = transaction->mObjectStoreNames.Length();
> +  if (count > 0) { // don't undeflow if count == 0
> +    for (uint32_t index = count - 1; index > 0; index--) {

Let's move count > 0 into the loop condition, and make it a const uint32_t.  That'll make this easier to read and the compiler should be smart enough not to check the condition every time through the loop.

Comment 4

[Jon Morton [:jmorton]]

Sounds reasonable, wasn't a fan of the check either. I'll change this.

Comment 5

[Jon Morton [:jmorton]]

Attachment: 0001-Bug-1013221-Remove-duplicate-object-store-names-when.patch (with review changes)

Comment 6

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

Comment on attachment 8430967 [details] [diff] [review]
0001-Bug-1013221-Remove-duplicate-object-store-names-when.patch (with review changes)

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

In the future, when I give you r+ with comments it means I trust you to make the changes I asked for without me looking at it again, and you can just set r+ on the patch yourself.

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a56b2d6457e

Comment 8

[David Fahlander]

When looking briefly at the diff I don't see that it handles the situation when the same store name is listed first and last but not in the middle. Example:
  ["users", "tickets", "users"]

Comment 9

[Jon Morton [:jmorton]]

Hi David,

If you look a little bit above the change (https://hg.mozilla.org/integration/mozilla-inbound/rev/9a56b2d6457e#l1.28) you'll see that the list is sorted before we check for duplicates, so it will work for the case you provided aswell.

Comment 10

[David Fahlander]

Ok I got it, thanks!

Comment 11

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

Btw, thanks for the bug report David.  You gave us exactly what we needed to track down and fix the problem.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9a56b2d6457e