Closed Bug 1102570 Opened 10 years ago Closed 10 years ago

IndexedDB storage is being closed too soon if invalidated

Categories

(Core :: Storage: IndexedDB, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- unaffected
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

References

Details

Attachments

(1 file)

Attached patch Patch, v1Splinter Review
Invalidate currently calls Close, and that tries to run the next synchronized op. We shouldn't let that happen until the background thread is actually done with it.
Attachment #8526398 - Flags: review?(Jan.Varga)
Comment on attachment 8526398 [details] [diff] [review]
Patch, v1

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

Hm, this doesn't make sense to me...
How can CloseOnMainThread() trigger next synchronized op ?
By calling OnStorageClosed() ?
OnStorageClosed() is needed just if you used AcquireExclusiveAccess() for a database, but that's not the case for IndexedDB anymore, since it does own exclusive access management.
Note that, DatabaseOfflineStorage::Close() in ActorsParent.cpp calls InvalidateOnMainThread() and that calls CloseOnMainThread(), so with this patch DatabaseOfflineStorage::Close() wouldn't set the closed state.

Could you just remove the OnStorageClosed() call ?

BTW, OnStorageClosed() along with AcquiteExclusiveAccess() for specific databases is removed in bug 771288 - patch "Remove unused stuff"
Comment on attachment 8526398 [details] [diff] [review]
Patch, v1

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

Ok, bent provided me some info and I now understand the problem.
Looks good, but we should get rid of this DatabaseOfflineStorage stuff :)
Attachment #8526398 - Flags: review?(Jan.Varga) → review+
https://hg.mozilla.org/mozilla-central/rev/03510730721a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment on attachment 8526398 [details] [diff] [review]
Patch, v1

Approval Request Comment
[Feature/regressing bug #]: 994190
[User impact if declined]: It's possible that test could randomly fail, and maybe clearing databases won't always succeed
[Describe test coverage new/current, TBPL]: This is a race, there's no real way to test it reliably
[Risks and why]: Low risk
[String/UUID change made/needed]: None
Attachment #8526398 - Flags: approval-mozilla-aurora?
Attachment #8526398 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.