Closed Bug 976608 Opened 6 years ago Closed 5 years ago

applicationCache corrupted due to concurrent updates running (should coalesce)

Categories

(Core :: Networking: Cache, defect, critical)

27 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: community, Assigned: mayhemer)

Details

Attachments

(2 files)

Note: This is about the cache used by <html manifest="manifest.appcache">. Excuse me if it's not appropriate here (we considered Core:Networking:Cache and Toolkit:Download Manager). Move as needed, please. Thank you very much.

After updating the manifest for an already cached app, I can trigger a "download process" by navigating to a resource which is part of the applicationCache.

During the download I can navigate to other resource, part of the same manifest. A second "download process" starts (I can see both in the httpd log).

If the "download process" fails (due to connectivity lost, for example) at the wrong time, the applicactionCache gets corrupted. The atomicity of the process is not honored and some files are cached incompletely. So, the app fails to work appropriately.

The atomicity is an essential part of the applicacionCache, so I think this should never happen. Also, the second "download process" is a waste of bandwidth.

Maybe a fail in the second "download process" is needed to trigger the bug. During my tests, a fail on the first one doesn't seems to be problematic.

I can reproduce this on Firefox 27.0.1 (windows) desktop and saw it on the wild in Firefox 26 for Android. As a test, I tried on an old Iceweasel 16.0.1 (linux) desktop and fails too, so this seems old.
See readme.html
Extract from the readme.html file:

* Go online.
* Install the test-app, navigating to appcache-install.html.
* When this test-app is installed/cached, you can navigate offline to "A" and "B" pages, and no alert message is shown, because everything is in place.
* Update the manifest.appcache file. For example, a new comment line at the end:
    echo "#" >> manifest.appcache
* Go online.
* Switch fast, for example, from to B and then to A, back to B, using the links provided. With the correct timming, this can trigger _two_ "download process"es for the only once updated manifest.appcache file.
* Cut the transmission in the middle of the second "download process". This tries to simulate the lost of connection of a mobile device. Not every cut triggers the bug, you have to win some race. For this test-app I think you have to cut in the middle of the padding_and_global_definition.js file. (I naively put socat between Firefox and the httpd; socat wrapped with trickle to slow the download, so I can C-c before everything is fast completed).
* Eventually, you'll hit the bug. The applicationCache is corrupted (maybe just one file? That's enough: atomicity, and correct old data are lost). The javascript 'test_object' is not defined anymore and alert("Oops.") is executed in accesses to a.html or b.html.
* Try again from "Update the manifest.appcache file", as necessary.
Product: Web Apps → Firefox
Version: unspecified → 27 Branch
Is this relevant?

https://mxr.mozilla.org/mozilla-central/source/dom/src/offline/nsDOMOfflineResourceList.cpp

385   // XXX: This is a race condition.  remove() is specced to remove
386   // from the currently associated application cache, but if this
387   // happens during an update (or after an update, if we haven't
388   // swapped yet), that remove() will be lost when the next update is
389   // finished.  Need to bring this issue up.
Component: General → Networking: Cache
Product: Firefox → Core
There is a code to coalesce appcache updates.  I know it has a flaw.  This should not be that hard to fix.
Assignee: nobody → honzab.moz
Summary: applicationCache corrupted → applicationCache corrupted due to concurrent updates running (should coalesce)
Attached patch v1Splinter Review
This is not a small change and could cause more harm than good.  We'll see...


 https://tbpl.mozilla.org/?tree=Try&rev=1add269a0b95
Attachment #8453646 - Flags: review?(jduell.mcbugs)
Status: NEW → ASSIGNED
Comment on attachment 8453646 [details] [diff] [review]
v1

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

Looks right.
Attachment #8453646 - Flags: review?(jduell.mcbugs) → review+
https://hg.mozilla.org/mozilla-central/rev/fafed32afbea
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.