Closed Bug 1118216 Opened 9 years ago Closed 9 years ago

crash in java.lang.IllegalStateException: This message cannot be recycled because it is still in use. at android.os.Message.recycle(Message.java)

Categories

(Firefox for Android Graveyard :: General, defect)

35 Branch
All
Android
defect
Not set
critical

Tracking

(firefox34 unaffected, firefox35 wontfix, firefox36 verified, firefox37 verified, firefox38 verified)

VERIFIED FIXED
Firefox 38
Tracking Status
firefox34 --- unaffected
firefox35 --- wontfix
firefox36 --- verified
firefox37 --- verified
firefox38 --- verified

People

(Reporter: u421692, Assigned: snorp)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-d072a8c1-e9f0-4a8c-8d02-5ca6b2150106.
=============================================================
Device: Nexus 5(Android 5.0.1); Firefox 35 Beta 10
I don't have STR for this issue, just browsing around when this happened.
From what I see in the crash reports, this is reproducing only on Android Lollipop, and most of the crashes are happening in Beta.
java.lang.IllegalStateException: This message cannot be recycled because it is still in use.
	at android.os.Message.recycle(Message.java:279)
	at org.mozilla.gecko.GeckoAppShell.pumpMessageLoop(GeckoAppShell.java:2447)
	at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:325)
	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:184)
I got this in Nightly today. I thought I had already fixed it, and it appears I tried in bug 1055166. I am not sure if this additional msg.recycle() snuck in or if I am just dumb.
Assignee: nobody → snorp
Attachment #8558523 - Flags: review?(rnewman)
Yeah I just flat out missed this other call in the initial patch. I blame the reviewer :)
Comment on attachment 8558523 [details] [diff] [review]
Catch IllegalStateException when recycling Messages

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

This change is fine, but:

https://groups.google.com/forum/?fromgroups#!topic/android-developers/9pHuc7lGunY

"When in doubt, don't call Message.recycle().  It is just an optimization.  The platform does this for you when it is done dispatching a message.

And yes: the platform does this for you when done dispatching a message, so when you handler, you no longer own the message and should not touch that object any more."


So I think it might be worth reconsidering whether we need to do this at all, assuming that all of our messages end up being dispatched.
Attachment #8558523 - Flags: review?(rnewman) → review+
(In reply to Richard Newman [:rnewman] from comment #5)
Now that you mention it, I think that's why I didn't change the other call here, because it never got dispatched. I wonder why it's throwing. I think maybe we do want to just stop calling recycle().
Comment on attachment 8558581 [details] [diff] [review]
Stop recycling Message instances, as it's unnecessary

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

Keep an eye on the log, wait until after a GC, make sure there aren't any scary messages about growing the message pool or finalizing messages.
Attachment #8558581 - Flags: review?(rnewman) → review+
https://hg.mozilla.org/mozilla-central/rev/05ecd4d258b2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
This looks like a pretty simple crash fix. As 37 is affected, what do you think about uplifting the fix for Beta 4 (gtb Mon, Mar 9)?
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #11)
> This looks like a pretty simple crash fix. As 37 is affected, what do you
> think about uplifting the fix for Beta 4 (gtb Mon, Mar 9)?

Seems fine to me
Flags: needinfo?(snorp)
Comment on attachment 8558581 [details] [diff] [review]
Stop recycling Message instances, as it's unnecessary

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: crashes
[Describe test coverage new/current, TreeHerder]: nightly, aurora
[Risks and why]: very low
[String/UUID change made/needed]: none
Attachment #8558581 - Flags: approval-mozilla-beta?
Comment on attachment 8558581 [details] [diff] [review]
Stop recycling Message instances, as it's unnecessary

Pretty simple crash fix that has been on m-c for a month. Let's get this into Beta 4. Beta+
Attachment #8558581 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Using the following STR's on Android 5.0.x, I was able to reproduce this crash on FF 36 Release and FF 37 Beta 2.
1. Go to intel.com/museumofme
2. Play the video in fullscreen 

Result: FF crashes

On FF 37 Beta 4, as well as on FF 38 an FF 39, when following this steps, FF becomes unresponsive
This issue was reported here: https://bugzilla.mozilla.org/show_bug.cgi?id=1140359
Status: RESOLVED → VERIFIED
I am reproducing this issue on Nexus 7(Android 5.0.2) on Firefox 36.0.2 while playing a video on youtube.com (desktop site).
I see that this crash is WONTFIX on FF 36, should we consider uplifting this to release branch, since it affects Youtube desktop site?
PS: On big tablets, desktop site is by default on Youtube.com.
Flags: needinfo?(lmandel)
On HTC Nexus 9 (Android 5.0.1) the crash reproduces only by tapping "tap to play" on flash content
Passing ni to Sylvestre, who is managing 36.
Flags: needinfo?(lmandel) → needinfo?(sledru)
This bug was present in 35 already, what changed (besides that we have a fix)? :) thanks
Flags: needinfo?(sledru)
Nothing has changed, we've realized that Firefox crashes every time when playing videos using flash on Android 5.0, and this has a big user impact. I was thinking it's an important issue for a release build.
Ok, I agree the impact is important but as we are going to release 37 soon, we already built 36.0.2 and we have this bug for a while.
James, we have to make a second build for 36.0.2 and we could take this patch. Do you think we should do it? If yes, could you fill the uplift request? Thanks
Flags: needinfo?(snorp)
Comment on attachment 8558581 [details] [diff] [review]
Stop recycling Message instances, as it's unnecessary

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: some crashes when using flash?
[Describe test coverage new/current, TreeHerder]: nightly, aurora
[Risks and why]: low
[String/UUID change made/needed]: none
Flags: needinfo?(snorp)
Attachment #8558581 - Flags: approval-mozilla-release?
Attachment #8558581 - Flags: approval-mozilla-release? → approval-mozilla-release+
Verified as fixed on Firefox 36.0.2 Build2 on Nexus 5 (Android 5.0.2) and Nexus 7 (Android 5.0.2).
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.