769893 - java.lang.IllegalArgumentException: Cannot draw recycled bitmaps at android.view.GLES20Canvas.drawBitmap(GLES20Canvas.java) on Jelly Bean (Galaxy Nexus and Nexus 7)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Scoobidiver (away)]

It's #21 top crasher in 14.0 but has been hit by only 4 users so far. Here is a crash report: bp-dfb3e08d-b942-46e5-ac75-03da12120630.

java.lang.IllegalArgumentException: Cannot draw recycled bitmaps
	at android.view.GLES20Canvas.drawBitmap(GLES20Canvas.java:778)
	at android.view.GLES20RecordingCanvas.drawBitmap(GLES20RecordingCanvas.java:117)
	at android.graphics.drawable.BitmapDrawable.draw(BitmapDrawable.java:393)
	at android.widget.ImageView.onDraw(ImageView.java:979)
	at android.view.View.draw(View.java:13458)
	at android.view.View.getDisplayList(View.java:12409)
	at android.view.View.getDisplayList(View.java:12453)
	at android.view.View.draw(View.java:13182)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2929)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2799)
	at android.view.View.draw(View.java:13461)
	at android.view.View.getDisplayList(View.java:12409)
	at android.view.View.getDisplayList(View.java:12453)
	at android.view.View.draw(View.java:13182)
	at android.view.ViewGroup.drawChild(ViewGroup.java:2929)
	at android.widget.ListView.drawChild(ListView.java:3226)
	at android.view.ViewGroup.dispatchDraw(ViewGroup.java:2799)
	at android.widget.AbsListView.dispatchDraw(AbsListView.java:2433)
	at android.widget.ListView.dispatchDraw(ListView.java:3221)
	at android.view.View.draw(View.java:13461)
	at android.widget.AbsListView.draw(AbsListView.java:3759)
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.IllegalArgumentException%3A+Cannot+draw+recycled+bitmaps+at+android.view.GLES20Canvas.drawBitmap%28GLES20Canvas.java%29

Comment 1

[Scoobidiver (away)]

It has appeared after Google I/O 2012.

Comment 2

[Scoobidiver (away)]

It's #14 top crasher in 14.0 and #2 in 14.0b10.

Comment 3

[Chris Peterson [:cpeterson]]

The Galaxy Nexus crashes look like they are still running ICS (Linux kernel 3.0.1), not Jelly Bean (Linux kernel 3.1.10).

Comment 4

[Chris Peterson [:cpeterson]]

sriram, do you have any insight into these recycled bitmap exceptions? Unfortunately, there are no Gecko functions in the stack traces.

The good news is that the crashes only seem to happen on Fennec 14 and 15, not 16.

Comment 5

[Scoobidiver (away)]

(In reply to Chris Peterson (:cpeterson) from comment #3)
> The Galaxy Nexus crashes look like they are still running ICS (Linux kernel
> 3.0.1), not Jelly Bean (Linux kernel 3.1.10).
The Linux kernel version is indeed between 3.0.31 and 3.1.10, but the OS is always something like google/takju/maguro:4.1/JRN84D/392829:user/release-keys and Jelly Bean is 4.1.

Comment 6

[Chris Peterson [:cpeterson]]

Scoobidiver, good eye. Thanks for pointing that out.

Comment 7

[Scoobidiver (away)]

(In reply to Chris Peterson (:cpeterson) from comment #4)
> The good news is that the crashes only seem to happen on Fennec 14 and 15,
> not 16.
So it was fixed before 16.0a1/20120630. The working window is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a7a905fd70d5&tochange=f08d285b63b0

Comment 9

[Scoobidiver (away)]

Bug 774107 contains STR.

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Assigning to kbrosnan for QA, for narrowing the regression range.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

I'm unable to reproduce this using the STR in bug 774107. Using the release Firefox on a Galaxy Nexus running 4.1.1. I opened a tab with planet.mozilla.org, a second tab with techcrunch.com, and a third tab with cnn.com. In between all of these I opened the tab panel and switched tabs many many times, but never got this exception.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

(I did all the tab-switching while the pages were still loading, as per the STR)

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Also I don't see any calls to Bitmap.recycle() anywhere in our code, so I can't really make a guess as to why this is occurring.

Comment 14

[François Marier [:francois]]

A few more examples of this problem on a Galaxy Nexus running Android 4.1.1 (Firefox Beta):

https://crash-stats.mozilla.com/report/index/bp-fe24362c-1ce3-4e14-b2cd-edc102120722
https://crash-stats.mozilla.com/report/index/bp-765fb1e5-548f-4c73-b240-c855c2120721
https://crash-stats.mozilla.com/report/index/bp-84f74e17-cbf2-442b-b5c2-c8b1c2120721
https://crash-stats.mozilla.com/report/index/bp-5e654c06-f0ae-46ce-bcbf-728462120721
https://crash-stats.mozilla.com/report/index/bp-ac5f5dfa-9bbe-4631-ac54-e011c2120720
https://crash-stats.mozilla.com/report/index/bp-10562bf7-2e0e-471e-8ea4-b28bc2120720
https://crash-stats.mozilla.com/report/index/bp-c82051bc-05fc-47e0-8fb8-392bf2120718
https://crash-stats.mozilla.com/report/index/bp-70d1a306-a148-4d87-ade4-674172120718

Comment 15

[Alex Keybl [:akeybl]]

This appears to be significantly affecting JB users (I've heard this from multiple sources). Tracking for our next release.

My girlfriend is able to reproduce on her Nexus intermittently, so worse come to worst we can steal her phone for the day. Let me know.

Comment 16

[Alex Keybl [:akeybl]]

And fyi her STR also involve opening up the tab panel while a page is loading.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

I was looking in the wrong place, apparently. In Beta's version of Tab.java there is this code:

                        Bitmap bitmap = Bitmap.createScaledBitmap(b, getThumbnailWidth(), getThumbnailHeight(), false);

                        if (mState == Tab.STATE_SUCCESS)
                            saveThumbnailToDB(new BitmapDrawable(bitmap));

                        mThumbnail = new BitmapDrawable(bitmap);
                        b.recycle();

createScaledBitmap can in some conditions just return the argument "b", which we are recycling. We could then end up trying to draw that bitmap scaled bitmap which has already been recycled. This code was removed in bug 755070 which landed in Firefox 16, which explains why this crash doesn't occur in 17.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (:kats) from comment #17)
> which explains why this crash doesn't occur in 17.

.. or 16 (presumably anybody running jelly bean and 16 is running the latest build of 16 which has the fix).

Comment 19

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch for mozilla-beta

This patch (which applies to Mozilla-beta) should fix this crash, if my theory is correct.

Comment 20

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 644951 [details] [diff] [review]
Patch for mozilla-beta

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: crashes when opening the tabs tray on jelly bean devices
Testing completed (on m-c, etc.): none (this patch doesn't apply to m-c or aurora)
Risk to taking this patch (and alternatives if risky): mobile only, should be pretty low risk. it should only affect the code path where we're hitting the crash.
String or UUID changes made by this patch: none

Comment 21

[Sriram Ramasubramanian [:sriram]]

This piece of code used to be there long long ago. Where did it go?

Comment 22

[Alex Keybl [:akeybl]]

Comment on attachment 644951 [details] [diff] [review]
Patch for mozilla-beta

[Triage Comment]
Thanks for the quick turnaround. Let's get this in for tomorrow's beta build.

Comment 23

[Kartikaya Gupta (email:[e-mail])]

Landed on beta:

https://hg.mozilla.org/releases/mozilla-beta/rev/150ee02206d6

Comment 24

[Kartikaya Gupta (email:[e-mail])]

I'll leave this open for now, will close in a week or so if the crash-stats confirm the fix.

Comment 25

[Chris Peterson [:cpeterson]]

Comment on attachment 644951 [details] [diff] [review]
Patch for mozilla-beta

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

::: mobile/android/base/Tab.java
@@ +166,5 @@
>  
>                          if (mState == Tab.STATE_SUCCESS)
>                              saveThumbnailToDB(new BitmapDrawable(bitmap));
>  
>                          mThumbnail = new BitmapDrawable(bitmap);

kats, why do we need to allocate separate BitmapDrawables for saveThumbnailToDB() and mThumbnail?

@@ +168,5 @@
>                              saveThumbnailToDB(new BitmapDrawable(bitmap));
>  
>                          mThumbnail = new BitmapDrawable(bitmap);
> +                        if (bitmap != b)
> +                            b.recycle();

Can BitmapDrawable's constructor throw OutOfMemoryError? Should we call b.recycle() in a finally {} block?

Comment 27

[Kartikaya Gupta (email:[e-mail])]

(In reply to Chris Peterson (:cpeterson) from comment #25)
> 
> kats, why do we need to allocate separate BitmapDrawables for
> saveThumbnailToDB() and mThumbnail?
> 

Good catch, I don't think we need two separate drawables. However since the drawable objects just wrap the bitmap I don't think it's very expensive. We can fix it in m-c.

> 
> Can BitmapDrawable's constructor throw OutOfMemoryError? Should we call
> b.recycle() in a finally {} block?

Ordinarily I would say yes, putting it in a finally block is probably a good idea, but that code exists only on mozilla-beta now so I'm not sure it's worth making that change.

Comment 28

[Kartikaya Gupta (email:[e-mail])]

I don't see any crash reports for this bug in 15.0b2, resolving as fixed.

Comment 29

[Josh Aas]

This crashes release Firefox on my Jelly Bean Galaxy Nexus a few times per day. Top crash number might not reflect the fact that this hits some users very hard and not others - for anyone with an affected device it's a huge problem. My friends are emailing me about it now too.

We should seriously consider this for a hotfix release. This is unacceptable to leave unfixed on a popular high-end device for many weeks.

Comment 30

[Kartikaya Gupta (email:[e-mail])]

I agree, this is now the topcrasher on 14.0.1 by a large margin (11.35% of all crashes), and is going to keep climbing as more people upgrade to JB.

Comment 31

[Alex Keybl [:akeybl]]

(In reply to Josh Aas (Mozilla Corporation) from comment #29)
> We should seriously consider this for a hotfix release. This is unacceptable
> to leave unfixed on a popular high-end device for many weeks.

We've asked the stability team to come up with a crashes/ADU count on JB, and will make a decision about a 14.0.2 accordingly. Hopefully we're able to make that call today/tomorrow.

Comment 32

[Alex Keybl [:akeybl]]

Comment on attachment 644951 [details] [diff] [review]
Patch for mozilla-beta

[Triage Comment]

[Triage Comment]
Based upon the latest crash volume, we've decided to take this fix in a mobile-only 14.0.2. Please land on mozilla-release as soon as possible.

Comment 33

[Kartikaya Gupta (email:[e-mail])]

Landed on the default branch of mozilla-release (thereby creating a new head in the repository):

https://hg.mozilla.org/releases/mozilla-release/rev/b96eb495bfe5

Comment 34

[Aaron Train [:aaronmt]]

(In reply to Josh Aas (Mozilla Corporation) from comment #29)
> This crashes release Firefox on my Jelly Bean Galaxy Nexus a few times per
> day.

Josh, can you verify if the fix above on mozilla-release works for you?

Comment 35

[Josh Aas]

(In reply to Aaron Train [:aaronmt] from comment #34)

> Josh, can you verify if the fix above on mozilla-release works for you?

Can you point me to an apk to test with?

Comment 36

[Scoobidiver (away)]

(In reply to Josh Aas (Mozilla Corporation) from comment #35)
> Can you point me to an apk to test with?
http://nightly.mozilla.org/

Comment 37

[Scoobidiver (away)]

(In reply to Josh Aas (Mozilla Corporation) from comment #35)
> Can you point me to an apk to test with?
ftp://ftp.mozilla.org/pub/mobile/candidates/14.0.2-candidates/build1/android/multi/fennec-14.0.2.multi.android-arm.apk

Comment 38

[Josh Aas]

I've been using the candidate build for a bit, haven't seen the crash.

Comment 39

[Lukas Blakk [:lsblakk] use ?needinfo]

We released a 14.0.2 to (only) JB users on August 7th - http://www.mozilla.org/en-US/mobile/14.0.2/releasenotes/