1905111 - IndexedDB indexes are sometimes not updates when a record is updated

Status: RESOLVED WORKSFORME

(Core :: Storage: IndexedDB, defect, P3)

Description

[me]

Steps to reproduce:

While I cannot reproduce this issue in an isolated, minimal test case, my web app at https://new.space is behaving differently after updating from 126 to 127 – sometimes when updating a record in IndexedDB, a request to an object store's index will incorrectly return the old record's information as the value.

Actual results:

My app has a little work queue, using a table as jobs, and marks them as processed: 1 after they are worked. The queue knows it's done when there are no more processed: 0 records.

However, after updating to 127 the work queue will end up in a loop working the same item over and over, since the index used to find the next processed: 0 record will return the old record over and over. I deleted the record, objectStore.get(key) returns an undefined result, and the index still returned the old record.

This happens with any sufficiently large account in my app, eventually the index in the work queue table starts returning old information and the work queue gets into a loop that never ends.

Expected results:

I expect updating or deleting a record to affect all indexes in the object store.

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Storage: IndexedDB' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Artur Iunusov]

Since, we have a minimal test case, could you, please, take a look, if this is related to the recent changes in IndexedDB?

Comment 3

[Jan Varga [:janv]]

Is this really S2 ?

Comment 4

[Artur Iunusov]

Is it possible for you to extract a minimal test case from the entire website? (https://new.space)
It will simplify debugging

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Or, could you perhaps try to find the regression window for the change in behavior using https://mozilla.github.io/mozregression/ ?

Comment 6

[me]

Hello. I am not able to extract a minimal test case yet, but I did use the mozregression tool (thank you for the link, I didn't know about this tool and it's incredible!) and I was able to bisect down to this commit:

https://phabricator.services.mozilla.com/D202947
“Check that ArrayBuffers in IDB keys are not detached. r=#dom-storage”

This sounds incredibly related: our app uses Uint8Array's as the keys for many of our tables and those are the tables that are now showing the problem where the indexes are not always updated when the records are updated or deleted. It is almost as if the record gets stuck in the index and can never be altered or removed after a certain point.

I added a screenshot of the mozregression run to this space: https://new.space/s/XzcPIWuybeEfEgzclNEafw#Ex8OJ50cCo4RfC6JUoC4nG9kC8ITGG-RxeYPm0TNoFk

(I'd love to send you a full log of the bisect in text, but I don't see how.)

Comment 7

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Setting based on comment #6.

BTW that patch invented its own BufferSource API to get Span, we shouldn't do that and rather use https://searchfox.org/mozilla-central/rev/6b17ef1f53b0bc68a0d746599c01ad8723c80880/dom/bindings/TypedArray.h#1014.

Comment 8

[Artur Iunusov]

Dear Jari,
I believe, after looking at the patch https://phabricator.services.mozilla.com/D202947 you will quickly find out, what effort is required to fix the issue. As well as the severity of this bug
Thanks in advance!

Comment 9

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

(I set it wrong)

Comment 10

[Jan Varga [:janv]]

Does your web app work with other browsers (latest versions).

Comment 11

[Jan Varga [:janv]]

(In reply to Jan Varga [:janv] from comment #10)

Does your web app work with other browsers (latest versions).

I'm asking because the patch which seems to be causing the regression, just fixed a bug where detached array buffers were allowed to be passed to IndexedDB. The patch changed that, so detached array buffers are not allowed anymore and this should be in line with the IndexedDB specification.

Comment 12

[me]

We have no issues in other browsers, only Firefox. We are planning a migration to string keys (encoding the array buffer), but no ETA on that so we are still seeing this issue.

Comment 13

[Jan Varga [:janv]]

Okay, thanks. There's actually a patch that will change some related code, which will likely land next week. It would be good if you could re-test your app with the latest Nightly version then. I'll let you know.

Comment 14

[me]

No worries, it's a big lift so it will take some time. And I can spin up any old version of the app on a subdomain to test anytime. I'll stick with this bug until it's closed 💪

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

I wonder if the problem could be the pre-emptive structured serialization copy we make when there are indices present interacting with the changes in that original patch.

Comment 16

[Jari Jalkanen [:jari]]

Attachment: WIP: Bug 1905111 - IDB indexes are sometimes not updated.

Comment 17

[Andrew Sutherland [:asuth] (he/him)]

Jari created a test case to attempt to repro, but it's not easily reproducing. Reporter, can you clarify what IDB objectStore and index in your app you are seeing the problem with and what we can do if using your app to try and maximize putting jobs in that queue? For example, if we set Firefox to "Work Offline" will this make it easier to accumulate a lot of state in the table?

Comment 18

[me]

I do not have steps to reproduce. We see the issue eventually in ever object store in our app. It does seem to happen sooner in object stores with more churn, but I don't have concrete data to prove that. I have tried to create a minimal test case to reproduce, but so far I have not been able to get one that shows the bug.

The indexes (including the main key one) in all our object stores eventually stop updating correctly, but it doesn't happen in the same place or after the same amount time. The way I found the commit where the regression happened was by using our web app with a very large amount of data until it errored (or didn't error, which I always tried twice to make sure it wasn't a fluke).

Comment 19

[Andrew Sutherland [:asuth] (he/him)]

(In reply to me from comment #18)

The indexes (including the main key one) in all our object stores eventually stop updating correctly, but it doesn't happen in the same place or after the same amount time. The way I found the commit where the regression happened was by using our web app with a very large amount of data until it errored (or didn't error, which I always tried twice to make sure it wasn't a fluke).

Thanks for the quick reply! 2 quick questions:

1. Given that it seems to impact multiple object stores, what's the easiest way for me to make a lot of data / values? Like, is just adding a lot of notes likely to cause the problem?
2. Can you clarify the idiom you are using to retrieve the contents of the indices? Specifically, like get/getAll/openCursor, and are you specifying an IDBKeyRange?

Comment 20

[me]

I have an account that is many months old and so it has a lot of spaces with lots of items and when I sign into it in Firefox that will download and process all those items and the events associated with them. I don't have a test account that is as large as mine to share, I'm sorry.

We process events (we call them activities) one at a time and use an index on an object store to easily get the next event to process and that usually is the object store that shows the problem first: we get the same event over and over. We update the event to mark it as processed, but the index is not updated and it returns the old record as the next event to process. This would cause Firefox to use a ton of CPU spinning forever, so we've already shipped code to keep a list of processed event IDs in memory so we can at least stop if this happens. I haven't been able to isolate it to a specific event or Uint8Array or anything like that, it's different every time.

We also see problems elsewhere like updating the name of a space in the spaces object store, get() with a Uint8Array key returns the old space even though the readwrite transaction to update the value at that key succeeded. Since the transactions are successful, we have to notice when Firefox is returning old information by sight to be like "oh, it's corrupt now." I have seen it in a corrupt state like this, returning the same old info for a record in the spaces object store no matter how many times I update it, and yet I am still able to update other records in the same object store. It's as if the key or value is poisoned.

We use a mix of get(), getAll(), and openCursor() throughout the app, and we do sometimes use an IDBKeyRange and yet sometimes we just get by key or just literally get all. So far I haven't seen anything consistent about when Firefox returns old data, other than when a value at a certain key is poisoned it can never be updated or deleted. When a key is corrupted, no matter how I try to get it (using get() or openCursor with a key range or anything else) it always returns a specific old value. getAll() will return the old value.

I'm sorry I don't have more info for you. I know how frustrating this bug report is, since it's not easily reproducible. We are currently migrating to only ever use string keys for object stores so our customers can use Firefox again, we are about 40% through that. I can run any tests in my large account that you like, test any custom builds, etc, just let me know.

Comment 21

[Andrew Sutherland [:asuth] (he/him)]

Thanks for the extra context!

I see that we haven't actually asked you to retest after the fix mentioned in comment 13 landed; can you re-test with a recent Firefox nightly build (specifically any nightly from July 20th or later)? Based on your characterization of the problem as effectively non-deterministic and bisection identifying the specific commit in question, it's quite likely for it to be the problem.

(In reply to me from comment #20)

I'm sorry I don't have more info for you. I know how frustrating this bug report is, since it's not easily reproducible. We are currently migrating to only ever use string keys for object stores so our customers can use Firefox again, we are about 40% through that. I can run any tests in my large account that you like, test any custom builds, etc, just let me know.

It's not your fault the bug is hard to reproduce! Thanks so much for making the effort to report the bug and your willingness to proactively assist us in investigations!

Comment 22

[Jari Jalkanen [:jari]]

Is it possible that the keys in the Uint8Array contain arbitrary, non-unicode byte sequences such as images, random numbers or hash values?

edit: removed misleading speculation

Comment 23

[me]

The keys we are using are just 16 random bytes. Basically a uuid v4, but just as the bytes. No images or anything, but also since they are random they are def not safe to interpret as strings.

I'm sorry I haven't tested the latest Firefox nightly yet. It's on my todo list and I'll get to it in just a bit.

Comment 24

[me]

I can confirm that Firefox Nightly 130.0a1 (2024-07-29) and (2024-07-30) do not reproduce the bad behavior. Using the same code locally I can reproduce in production Firefox easily (signing into a very large account), in Nightly after repeated attempts I cannot reproduce.

I also tested Firefox Nightly 130.0a1 (2024-07-30) against our current staging environment with a very large account repeatedly and it will not reproduce the issue. Should I resolve this issue myself as "WORKSFORME"?

Comment 25

[Jan Varga [:janv]]

Ok, thanks for the confirmation. The patch from comment 13 has been uplifted to current beta (FF 129) and esr 128 in the meantime. The release date for FF 129 is August 6.

Comment 26

[Jan Varga [:janv]]

(In reply to me from comment #24)

I also tested Firefox Nightly 130.0a1 (2024-07-30) against our current staging environment with a very large account repeatedly and it will not reproduce the issue. Should I resolve this issue myself as "WORKSFORME"?

Yes, please go ahead and resolve it as "WORKSFORME". Thanks for all the information and extra testing.