Closed Bug 1169994 Opened 6 years ago Closed 6 years ago

DOM cache may unregister offline storage too soon

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: janv, Assigned: bkelly)

Details

Attachments

(2 files)

Attached patch slowdown patchSplinter Review
After the vacuum stuff landed, closing of storage connections takes a bit more time. This causes that my directory lock stuff doesn't pass tests anymore. I investigated the issues a bit and I believe the offline storage is unregistered too soon. The same issue arises on current m-c when I artificially slowdown the closing even more (see the patch).

Steps to reproduce:
1. apply the slowdown patch
2. run ./mach mochitest-plain dom/cache dom/indexedDB
3. see the assertion in OriginInfo::~OriginInfo

I think Context::~Context should do a better job and release mOfflineStorage when the connection actually finished the closing procedure.
The assertion in OriginInfo destructor means that QM deleted an origin that still has live quota objects. I verified that these quota objects belong to cache.sqlite and cache.sqlite-wal
Please post the patch that enables your directory locking.  The ~Context() destructor should not run while the Connection is executing on the IO thread.
Flags: needinfo?(Jan.Varga)
Oh, I see.  It happens without your patch.  Let me look.
Flags: needinfo?(Jan.Varga)
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Shorter set of tests to run to trigger:

  ./mach mochitest dom/cache/test/mochitest/test_caches.html dom/indexedDB/test/test_add_put.html
(In reply to Ben Kelly [:bkelly] from comment #4)
> Shorter set of tests to run to trigger:
> 
>   ./mach mochitest dom/cache/test/mochitest/test_caches.html
> dom/indexedDB/test/test_add_put.html

Yeah, that narrows it down even more.
The Context's QuotaInitRunnable was not dropping its mData ref on the target thread.  This resulted in the connection getting proxy released when the init was canceled.

This patch fixes the runnable to drop the ref correctly.  I also changed the proxy release to a thread assert.
Attachment #8616907 - Flags: review?(ehsan)
Attachment #8616907 - Flags: review?(ehsan) → review+
New try since the outage caused the last to fail:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=474b3eaf4093
https://hg.mozilla.org/mozilla-central/rev/168cdb48c5eb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.