Closed Bug 612703 Opened 9 years ago Closed 9 years ago

Downloading a 1 gig file will cause the Droid 2 to run out of memory and reboot

Categories

(Firefox for Android Graveyard :: General, defect, critical)

ARM
Android
defect
Not set
critical

Tracking

(fennec2.0b3+)

VERIFIED FIXED
Tracking Status
fennec 2.0b3+ ---

People

(Reporter: nhirata, Assigned: wesj)

Details

Attachments

(2 files, 4 obsolete files)

1. go to the app store and install SysMonitor
2. go to http://www.thinkbroadband.com/download.html
3. long tap the 1 gb file and save
4. open the System Notifications
5. wait

Expected: the device would say that there's not enough space
Actual: the device will try to download and reboot after a while.
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101116 Firefox/4.0b8pre Fennec/4.0b3pre
Wes, do we get the right file size? Does getAvailableSpace just not work?
Assignee: nobody → wjohnston
I do see a number coming from getAvailableSpace that seems reasonable. Trying to reproduce...
Group: mozilla-confidential
why is this Mozilla confidential?
Not sure. If I did that it was an unintentional click.

I did eventually see the device reboot, even though I have plenty of space available on my SD card. (> 14GB)
Group: mozilla-confidential
Severity: normal → critical
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0-
tracking-fennec: 2.0- → 2.0b4+
Attached file Logcat
Attaching a logcat report from the download. There's probably a lot there we don't care about, but I wanted to save it somewhere. At a first glance, this looks like an OOM (not disk space) problem to me.
This is likely due to progressbars in the notification we are using. A simple script that just creates a progress notification and increments it a whole bunch over a few minutes will also eventually freeze. Likewise, there are some reports on stack overflow about similar problems:

http://stackoverflow.com/questions/3390280/android-memory-leak-in-notification-service
http://stackoverflow.com/questions/3608684/huge-memory-usage-in-notifications

Still building to test. If this is the problem, maybe we limit updates to the notifications (i.e. only update the bar if the change is greater than x%)?
Attached patch Patch v1 (obsolete) — Splinter Review
This changes the progress bars to only update when a certain percentage change has occurred (1% here), or when the text changes (trying to make sure we support changing the text without changing the percentage and vice versa).

I have run some simple tests with faked progress bars here and this seems fine. Trying a download with a larger file (ubuntu iso) now.
Attachment #494055 - Flags: review?(blassey.bugs)
Good catch!

From those links it seems this is indeed a bug in the Android OS. I can't find a bug in their tracker for it though, perhaps we should file one, once we are sure that is the issue here.
Comment on attachment 494055 [details] [diff] [review]
Patch v1


>         String text;
>+        int percent = 0;
>+        if (aProgressMax > 0)
>+            percent = (int)(100 * aProgress / aProgressMax);

While you're here, please use NumberFormat.getPercentInstance().format(aProgress / aProgressMax); 

r+ other than that, but I want to double check your formatting code
Attachment #494055 - Flags: review?(blassey.bugs) → review-
Comment on attachment 494055 [details] [diff] [review]
Patch v1


>             text = percent + "%";
err.... I meant use that here. I think percent should be a double for it to work though.
Attached file Patch v2 (obsolete) —
Revised to use NumberFormat stuff.
Attachment #494055 - Attachment is obsolete: true
Attachment #494404 - Flags: review?(blassey.bugs)
Attached patch Patch v2 again (obsolete) — Splinter Review
Arrgh. Forgot to mark as a patch.
Attachment #494404 - Attachment is obsolete: true
Attachment #494406 - Flags: review?(blassey.bugs)
Attachment #494404 - Flags: review?(blassey.bugs)
Comment on attachment 494406 [details] [diff] [review]
Patch v2 again

as discussed on irc, percent ranges from 0 to 1, but the threshold is 1
Attachment #494406 - Flags: review?(blassey.bugs) → review-
Attached patch Patch v3 (obsolete) — Splinter Review
Sorry bout that. Testing this right now, but it has been running for around 15 minutes (the numbers update and everything now) and my system is still responsive.
Attachment #494406 - Attachment is obsolete: true
Attachment #494448 - Flags: review?(blassey.bugs)
Comment on attachment 494448 [details] [diff] [review]
Patch v3


>+        if (!mPrevAlertText.equals(text) || Math.abs(mPrevPercent - percent) >= UPDATE_THRESHOLD) {

one last nit, just return early here
Attachment #494448 - Flags: review?(blassey.bugs) → review+
Attached patch Fix v4Splinter Review
Reversed the logic of the if. Tested again with my little manual script.
Attachment #494448 - Attachment is obsolete: true
Attachment #494470 - Flags: review+
tracking-fennec: 2.0b4+ → 2.0b3+
http://hg.mozilla.org/mozilla-central/rev/5ae1f2fa0d9f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
I can't verify this bug because I get "The connection has timed out" page.. Does someone else see this too?
I verified this on Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110911 Firefox/9.0a1 Fennec/9.0a1.
1. The device is not rebooted anymore
2. The message saying that "there's not enough space" is still missing. Instead, an empty message box is displayed.(for this issue, bug #686286 was logged)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.