1569312 - Crash in [@ java.lang.OutOfMemoryError: at dalvik.system.VMRuntime.newNonMovableArray(Native Method)]

Status: VERIFIED FIXED

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

Description

[Marcia Knous [:marcia]]

This bug is for crash report bp-7404c18c-b392-4521-822c-fffe40190726.

We did some work in another bug (Bug 1507427), but this crash is still hanging around in Fennec 68 release at #16: https://mzl.la/2yg8J8S. This crash also affects Fenix: https://mzl.la/30WNN30

(67.11% in signature vs 06.11% overall) android_version = 24 (REL)
29.82% in signature vs 00.91% overall) adapter_driver_version_clean = OpenGL ES 3.2 v1.r15p0-00rel0.68b65ac7cf15907aeb95fa944f39eef2 [89.54% vs 28.67% if adapter_device_id = Mali-T760]
(29.82% in signature vs 00.91% overall) adapter_driver_version = OpenGL ES 3.2 v1.r15p0-00rel0.68b65ac7cf15907aeb95fa944f39eef2 [89.54% vs 28.67% if adapter_device_id = Mali-T760]

Java stack trace:

java.lang.OutOfMemoryError
	at dalvik.system.VMRuntime.newNonMovableArray(Native Method)
	at android.graphics.Bitmap.nativeCreate(Native Method)
	at android.graphics.Bitmap.createBitmap(Bitmap.java:977)
	at android.graphics.Bitmap.createBitmap(Bitmap.java:948)
	at android.graphics.Bitmap.createBitmap(Bitmap.java:915)
	at org.mozilla.gecko.BrowserApp.getBitmapOfToolbarChrome(BrowserApp.java:1668)
	at org.mozilla.geckoview.DynamicToolbarAnimator.handleToolbarAnimatorMessage(DynamicToolbarAnimator.java:171)
	at org.mozilla.geckoview.GeckoSession.handleCompositorMessage(GeckoSession.java:4855)
	at org.mozilla.geckoview.GeckoSession$Compositor.recvToolbarAnimatorMessage(GeckoSession.java:225)
	at org.mozilla.gecko.GeckoThread.runUiThreadCallback(Native Method)
	at org.mozilla.gecko.GeckoThread$1.run(GeckoThread.java:115)
	at android.os.Handler.handleCallback(Handler.java:751)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:154)
	at android.app.ActivityThread.main(ActivityThread.java:6682)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1520)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1410)

Comment 1

[Chris Peterson [:cpeterson]]

Emily, you worked on GV's screenshot code. Do you want to audit GV's use of screenshot Bitmaps? Who on the Fenix or A-C team is calling GV's screenshot API? I can ask them to audit the Fenix/A-C code.

The BrowserApp signature is Fennec only, but some of these crash signatures are from Fenix, e.g. bp-271c0f40-2afd-41d8-9fa2-516ff0190801.

James suspects Fenix is taking page screenshots more often than it needs. Is Fenix leaking GV's screenshot Bitmaps? Can Fenix or GV recycle the same Bitmap for each screenshot to avoid memory? That would reduce memory pressure, but might surprise the API callers if their previous screenshot Bitmaps change underneath them.

The crash spiked in mid-July, which is when we released ARM64 Fennec 68. It's surprising that we would see more more OOMs on ARM64. Perhaps the screenshot Bitmaps are larger on ARM64?

Comment 2

[Marcia Knous [:marcia]]

This is fairly high on Fennec 68 release - between 68 and 68.0.2 we are almost at 8K crashes. Petru - any further ideas here as to how we could improve the crash situation here for Fennec?

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Looking into possible causes.

Comment 4

[Chris Peterson [:cpeterson]]

The BrowserApp signature is Fennec only, but we also see this signature in Fenix, e.g. bp-271c0f40-2afd-41d8-9fa2-516ff0190801.

The GV team's theory is that Fenix is probably taking page screenshots more often than they need. Is Fenix destroying the screenshot Bitmaps? Do we reuse the Bitmap? But would that screenshot theory explain why Fennec is affected? There were over 1500 Fennec crash reports with this signature last week.

The crash spiked in mid-July, which is when we released ARM64 Fennec 68. It's surprising that we would see more more OOMs on ARM64.

See a similar stack trace from Fenix crash bug 1574992:

java.lang.RuntimeException
	at android.graphics.Bitmap.copyPixelsFromBuffer(Bitmap.java:617)
	at org.mozilla.gecko.GeckoThread.runUiThreadCallback(Native Method)
	at org.mozilla.gecko.GeckoThread$1.run(GeckoThread.java:2)
	at android.os.Handler.handleCallback(Handler.java:794)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:176)
	at android.app.ActivityThread.main(ActivityThread.java:6662)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:547)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:873)

Comment 6

[Marcia Knous [:marcia]]

Chris: I suggest we break this out into two separate bugs, keep this one for Fennec and file a new one for the Fenix issue, especially since it appears as the #2 crash in crash stats: https://crash-stats.mozilla.com/topcrashers/?product=Fenix&version=69.0b0&process_type=any and has a different stack. If that works for you, I can file the new bug.

Comment 7

[Chris Peterson [:cpeterson]]

(In reply to Marcia Knous [:marcia - needinfo? me] from comment #6)

Chris: I suggest we break this out into two separate bugs, keep this one for Fennec and file a new one for the Fenix issue, especially since it appears as the #2 crash in crash stats: https://crash-stats.mozilla.com/topcrashers/?product=Fenix&version=69.0b0&process_type=any and has a different stack. If that works for you, I can file the new bug.

SGTM. If you can file a new bug to track the Fenix signature, that would be helpful.

Is there a way for Socorro to recognize that Fennec crash reports with this signature should be point to this bug and Fenix crash reports with the similar signature should point to your new Fenix bug?

Comment 8

[Marcia Knous [:marcia]]

Bug 1577192 is the new one created for Fenix. Removing the ni for Emily in this bug.

AFAIK Socorro has no way to differentiate between crashes that happen on various products.

Comment 9

[Marcia Knous [:marcia]]

For Fennec, there is still a strong correlation to API 24:

(69.17% in signature vs 05.63% overall) android_version = 24 (REL)

A few of the comments mention having quite a few tabs:

• lots of open tabs and limited free space.
• I obviously exceeded the max # of tabs. is this documented anywhere?
• I was switching to an already open tab after typing a portion of the URL. I always have over 100 tabs open

Top crashing devices is SM-G920F, followed by Redmi Note 4.

Comment 10

[Petru-Mugurel Lingurar [:petru]]

Thanks Marcia for watching this!

Although there are no clear offenders I've checked again where could we improve things with minimal chances of regressions and I've found 3 places, one in particular that can be optimized for better Bitmap usages in the hope of easing the overall memory pressure.

• DynamicToolbarAnimator.handleToolbarAnimatorMessage which appears in comment#0 could indeed be the worst offender as that would create a new 1MB - 2MB sized Bitmap everytime the user scrolls the webpage down and animate the awesomebar up, outside the screen bounds.
  This would correlate with the above reports about having multiple tabs open. If the users got the awesomebar hidden that would mean an 1MB-2MB of in-memory Bitmaps for each tab.
• GeckoMediaControlAgent.generateCoverArt which generates a new image to be shown in the notification for when playing media can again be optimized.
• CanvasDelegate.draw which creates small bitmaps to do low level painting of various toolbar elements can again be optimized.

I think an effort to minimize the relatively high number of crashes on Release should make it up to ESR.
Updated the tracking flags to appear in the ESR 68 list of tickets.

Comment 11

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1569312 - Recycle Bitmaps to to ease memory pressure; r?Vladbaicu

The recommended optimization - BitmapFactory.Options.inBitmap for API 11+ could
not be used as that is only available for
"decode methods that take the Options object".
Used Bitmap.recycle() though in an effort to reduce memory pressure and allow
the app gc the Bitmap object and reclaim memory as soon as possible.

Comment 12

[Petru-Mugurel Lingurar [:petru]]

Try build with my patch applied on top of esr68 - https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2f3340507092aa3a5af85e82bb2470cce086e24
Please help test that the 3 scenarios from comment#10 work as before.

Comment 13

[Diana Rus]

Hi, I verified with the provided try build from comment 12, with Sony Xperia Z5 (Android 7.0) and Samsung Galaxy S6 (Android 6.0.1) the scenarios mentioned below. All of the function below functionalities work correctly besides does mentioned in notes.
On architecture aarch 64/arm64 and api16

• dynamic toolbar - scrolling down the toolbar is not displayed when "Full-screen browsing" is enabled and displayed when disabled
• dynamic toolbar in reader mode
• video notification when video is played with: youtube, dailymotion and rotten tomatoes
• and buttons for normal browsing, private browsing and adding a new tab buttons/icons.

Note:

• Video notification for rotten tomatoes for both devices on both architectures does not have the favicon correctly displayed - screenshot as for youtube notification - screenshot
• Video notification for daily motion for Samsung Galaxy S6 (Android 6.0.1) does not have the favicon correctly displayed on architecture aarch 64/arm64 - screenshot

Comment 14

[Petru-Mugurel Lingurar [:petru]]

Thank you Diana!
Seems to me like things are working just as before and this is what I needed confirmation for.

Comment 15

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9098763 [details]
Bug 1569312 - Recycle Bitmaps to to ease memory pressure; r?Vladbaicu

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Speculative fix for easing app's memory pressure.
• User impact if declined: Potential OOM crashes for which not optimum bitmap manipulation also contributes.
• Fix Landed on Version: 68
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small changes, no logic changed, confirmed no clear regressions by QA.
• String or UUID changes made by this patch:

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9098763 [details]
Bug 1569312 - Recycle Bitmaps to to ease memory pressure; r?Vladbaicu

Hopeful Fennec crash fix. Approved for 68.2b6.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/2cd4761fba04

Comment 18

[Chris Peterson [:cpeterson]]

This Bitmap fix will ship in Fennec ESR 68.2.

Comment 19

[Diana Rus]

Hi, verified with api 16 and aarch64/arm64 with Sony Xperia Z5 (Android 7.0), Samsung Galaxy S6 (Android 6.0.1) and Samsung Galaxy S8+ (Android 8) on Firefox Beta 68.2b6, the functionalities work correctly.
The mentioned outcomes from Notes - comment 13, seem to be present on older version of firefox as well did not come with this ticket.
I will leave the firefox-esr68 status unchanged until the fix reaches version 68.2.

Comment 20

[Stefan Deiac]

Hello, I can confirm that this issue is verified on Firefox 68.2.0 using Xiaomi Mi 8 Lite (Android 9), OnePlus 5T(Android 9) and LG G7 fit (Android 8.1).
Due to my findings, I'll mark this issue as verified and remove the qe-verify flag, thanks.