Closed Bug 827896 Opened 7 years ago Closed 7 years ago

Powering off the device during a download update deletes the partial download

Categories

(Toolkit :: Application Update, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: rik, Assigned: dhylands)

Details

Attachments

(3 files, 1 obsolete file)

Attached file Logcat during shutdown
Steps to reproduce:
1) Start a system update
2) Turn off the phone via the UI while downloading
3) Turn on the phone
4) Check for updates
5) Restart the update

Actual results:
The partial download is deleted and we have to download all the update

Expected results:
We continue downloading where it stopped

See logcat attachment with messages during the shutdown phase.
Anthony, is it still happening ?

If yes I believe this is a dupe of one of the system update bugs that we already have.
Yes, it still happens.
Dave, what do you think of this bug ?
Flags: needinfo?(dhylands)
So I think that we need some more information.

I've tested a scenario similar to this, although I was doing

stop b2g

rather than powering down the device. It would be useful to get a logcat when the phone boots up after the powerdown. It would also be useful to see the contents of:

/data/local/updates/0/update.status

If update.status doesn't contain the string "downloading" then that suggests that the update.status file isn't being flushed to disk after starting the download.

If the update.status file does contain the string "downloading" then it should be resuming the partially downloaded file.
Flags: needinfo?(dhylands)
Dave, see step 2, we're stopping the phone in a "clean" way.

Here's an extract of logcat while the phone is shutting down:
AUS:SVC Downloader:onProgress - progress: 4733836/51166855
AUS:SVC Downloader:onStopRequest - original URI spec: http://update.boot2gecko.org/nightly/b2g_update_20130128070201.mar?build_id=20130128070201&version=18.0, final URI spec: http://update.boot2gecko.org/nightly/b2g_update_20130128070201.mar?build_id=20130128070201&version=18.0, status: 2147746065
AUS:SVC Downloader:onStopRequest - status: 2147746065, current fail: 0, max fail: 20, retryTimeout: 30000
AUS:SVC Downloader:onStopRequest - non-verification failure
AUS:SVC getStatusTextFromCode - transfer error: Failed (unknown reason), default code: 2152398849
AUS:SVC getFileFromUpdateLink linkFile.path: /data/local/updates/0/update.link, link: /mnt/sdcard/updates/0/update.mar
AUS:SVC Downloader:onStopRequest - setting state to: download-failed
AUS:SVC Downloader:onStopRequest - all update patch downloads failed


And after booting, /data/local/updates/0/update.status doesn't exist.
(In reply to Anthony Ricaud (:rik) from comment #5)
> Dave, see step 2, we're stopping the phone in a "clean" way.
> 
> Here's an extract of logcat while the phone is shutting down:
> AUS:SVC Downloader:onProgress - progress: 4733836/51166855
> AUS:SVC Downloader:onStopRequest - original URI spec:
> http://update.boot2gecko.org/nightly/b2g_update_20130128070201.
> mar?build_id=20130128070201&version=18.0, final URI spec:
> http://update.boot2gecko.org/nightly/b2g_update_20130128070201.
> mar?build_id=20130128070201&version=18.0, status: 2147746065
> AUS:SVC Downloader:onStopRequest - status: 2147746065, current fail: 0, max
> fail: 20, retryTimeout: 30000

This is the key to the problem. onStopRequest is getting called with an error.
2147746065 = NS_ERROR_NOT_AVAILABLE

I'll take a look and see what's going on.

> AUS:SVC Downloader:onStopRequest - non-verification failure
> AUS:SVC getStatusTextFromCode - transfer error: Failed (unknown reason),
> default code: 2152398849
> AUS:SVC getFileFromUpdateLink linkFile.path:
> /data/local/updates/0/update.link, link: /mnt/sdcard/updates/0/update.mar
> AUS:SVC Downloader:onStopRequest - setting state to: download-failed
> AUS:SVC Downloader:onStopRequest - all update patch downloads failed

I'm pretty sure that the fact the it thinks the download failed is what ultimately causes the problem.
The NS_ERROR_NOT_AVAILABLE originates from the nsHttpChannel::OnProxyAvailable function here:
https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#4578
One way of dealing with this would to add NS_ERROR_NOT_AVAILABLE in Downloader_onStopRequest as a non-fatal failure.

Perhaps treating it like NS_ERROR_ABORT like this:
https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#3839

Adding needinfo for rstrong to see if this is a reasonable way to handle this.
Flags: needinfo?(robert.bugzilla)
That is normally handled during xpcom-shutdown
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1740

by the following call
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1747

Is that not possible?

If not, that should be reasonable though without knowing the other cases where NS_ERROR_NOT_AVAILABLE is returned it might end up taking that path when it shouldn't so that should be audited first and I think it would only be applicable to GONK.
Flags: needinfo?(robert.bugzilla)
(In reply to Robert Strong [:rstrong] (do not email) from comment #9)
> That is normally handled during xpcom-shutdown
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> nsUpdateService.js#1740
> 
> by the following call
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> nsUpdateService.js#1747
> 
> Is that not possible?

It seems that powering off does doesn't send an xpcom-shutdown, but rather calls SyncProfile, which sends a profile-change-net-teardown.
https://mxr.mozilla.org/mozilla-central/source/dom/power/PowerManagerService.cpp#126
https://mxr.mozilla.org/mozilla-central/source/dom/power/PowerManagerService.cpp#103

This profile-change-net-teardown seems to be what causes mHandlerActive to be set to false:
https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#1675
which, later on causes NS_ERROR_NOT_AVAILABLE (gHttpHandler->Active() is returning mHandlerActive)
https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#4578

Perhaps then, the right thing to do is to also listen for profile-change-net-teardown (where nsUpdateService.js is currently listening for xpcom-shutdown) and do the same thing as xpcom-shutdown is doing.

Looking at the places which send profile-change-net-teardown, they all seem to be shutdown related, except for one:
https://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsNativeAppSupportQt.cpp#64

PowerManagerService::SyncProfile (which is where gonk sends this) is currently called for PowerOff, Restart, and Reboot.

I'm happy to make that gonk-specific (registering for the profile-change-net-teardown notification).
That sounds better though you should make sure that nsUpdateService.js might get this before nsHttpHandler.cpp. Also, I am a tad concerned there might be other components that expect xpcom-shutdown and aren't shutting down cleanly because of it.
Treat the profile-change-net-teardown in the same manner as xpcom-shutdown. This causes an ongoing download to be cancelled, rather than fail.

If the download is cancelled, then it will be resumable once the phone boots up.

Since UpdateService is created before the http channel that's involved in the download, our profile-change-net-teardown callback will be called before the http-channel one (which would call OnStopRequest with NS_ERROR_NOT_AVAILABLE and cause the download to fail).
Cleaned up the comments.
Attachment #711599 - Attachment is obsolete: true
Attachment #711602 - Flags: review?(robert.bugzilla)
Assignee: nobody → dhylands
Component: Gaia::System → Application Update
Product: Boot2Gecko → Toolkit
Attachment #711602 - Flags: review?(robert.bugzilla) → review+
https://hg.mozilla.org/mozilla-central/rev/70f30a267a8b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.