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)
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)
1.94 KB,
patch
|
rnewman
:
review+
lmandel
:
approval-mozilla-beta+
Sylvestre
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
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.
Comment 1•9 years ago
|
||
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)
Assignee | ||
Comment 2•9 years ago
|
||
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 | ||
Comment 3•9 years ago
|
||
Assignee: nobody → snorp
Attachment #8558523 -
Flags: review?(rnewman)
Assignee | ||
Comment 4•9 years ago
|
||
Yeah I just flat out missed this other call in the initial patch. I blame the reviewer :)
Comment 5•9 years ago
|
||
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+
Assignee | ||
Comment 6•9 years ago
|
||
(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().
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8558523 -
Attachment is obsolete: true
Attachment #8558581 -
Flags: review?(rnewman)
Comment 8•9 years ago
|
||
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+
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/05ecd4d258b2
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/05ecd4d258b2
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Comment 11•9 years ago
|
||
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)?
Assignee | ||
Comment 12•9 years ago
|
||
(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)
Assignee | ||
Comment 13•9 years ago
|
||
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 14•9 years ago
|
||
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+
Comment 16•9 years ago
|
||
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
Reporter | ||
Comment 17•9 years ago
|
||
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)
Reporter | ||
Comment 18•9 years ago
|
||
On HTC Nexus 9 (Android 5.0.1) the crash reproduces only by tapping "tap to play" on flash content
Comment 19•9 years ago
|
||
Passing ni to Sylvestre, who is managing 36.
Flags: needinfo?(lmandel) → needinfo?(sledru)
Comment 20•9 years ago
|
||
This bug was present in 35 already, what changed (besides that we have a fix)? :) thanks
Flags: needinfo?(sledru)
Reporter | ||
Comment 21•9 years ago
|
||
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.
Comment 22•9 years ago
|
||
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.
Comment 23•9 years ago
|
||
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)
Assignee | ||
Comment 24•9 years ago
|
||
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?
Updated•9 years ago
|
Updated•9 years ago
|
Attachment #8558581 -
Flags: approval-mozilla-release? → approval-mozilla-release+
Reporter | ||
Comment 26•9 years ago
|
||
Verified as fixed on Firefox 36.0.2 Build2 on Nexus 5 (Android 5.0.2) and Nexus 7 (Android 5.0.2).
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•