Closed
Bug 784360
Opened 12 years ago
Closed 11 years ago
java.util.ConcurrentModificationException: at java.util.HashMap$HashIterator.nextEntry(HashMap.java) at org.mozilla.gecko.DoorHangerPopup.onTabChanged(DoorHangerPopup.java)
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox17+ verified, firefox18 verified)
VERIFIED
FIXED
Firefox 19
People
(Reporter: scoobidiver, Assigned: Margaret)
References
Details
(Keywords: crash, regression, topcrash, Whiteboard: [native-crash])
Crash Data
Attachments
(1 file)
1.81 KB,
patch
|
wesj
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
It first appeared in 17.0a1/20120806. The regression range is likely: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9453cf029b72&tochange=c8d94fe7506a It's likely a regression from bug 732336. Here is a crash report: bp-28152574-c339-4b59-8b99-62f822120821. java.util.ConcurrentModificationException at java.util.HashMap$HashIterator.nextEntry(HashMap.java:796) at java.util.HashMap$KeyIterator.next(HashMap.java:823) at org.mozilla.gecko.DoorHangerPopup.onTabChanged(DoorHangerPopup.java:99) at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:369) at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:358) at org.mozilla.gecko.Tabs$3.run(Tabs.java:183) at android.os.Handler.handleCallback(Handler.java:587) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:143) at android.app.ActivityThread.main(ActivityThread.java:4196) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:507) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:839) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:597) at dalvik.system.NativeStart.main(Native Method) More reports at: https://crash-stats.mozilla.com/report/list?signature=java.util.ConcurrentModificationException%3A+at+java.util.HashMap%24HashIterator.nextEntry%28HashMap.java%29
Reporter | ||
Updated•12 years ago
|
Whiteboard: [native-crash]
Assignee | ||
Comment 1•12 years ago
|
||
Sigh, threads. I'm not sure if this is the smartest way to avoid a ConcurrentModificationException, but it's what we've done elsewhere. There are also things like CopyOnWriteArrayList, but that feels too heavyweight for this case.
Assignee: nobody → margaret.leibovic
Attachment #656673 -
Flags: review?(wjohnston)
Comment 2•12 years ago
|
||
Comment on attachment 656673 [details] [diff] [review] patch Review of attachment 656673 [details] [diff] [review]: ----------------------------------------------------------------- I think it'd be cleaner to just use synchronized here. We aren't going to be doing so much with this list that I expect that to happen often. It matches well with the advice from here: http://developer.android.com/reference/java/util/concurrent/package-summary.html "A CopyOnWriteArrayList is preferable to a synchronized ArrayList when the expected number of reads and traversals greatly outnumber the number of updates to a list."
Updated•12 years ago
|
Attachment #656673 -
Flags: review?(wjohnston) → review-
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #2) > I think it'd be cleaner to just use synchronized here. Where would we use synchronized? I see above that I mentioned threads, but thinking about it more, the problem isn't that this is being accessed from multiple threads, the problem is that we're modifying the array as we're iterating over it. I've found I can reliably reproduce this crash if I close any tab that has multiple doorhangers (a good testcase is http://people.mozilla.com/~mwargers/tests/dom/multiple_doorhangers.htm).
Reporter | ||
Comment 5•12 years ago
|
||
It's #8 top crasher in 17.0b1.
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 656673 [details] [diff] [review] patch Wes, can you re-review this? See comment 3.
Attachment #656673 -
Flags: review- → review?(wjohnston)
Comment 7•12 years ago
|
||
Comment on attachment 656673 [details] [diff] [review] patch Review of attachment 656673 [details] [diff] [review]: ----------------------------------------------------------------- Ok. I can live with this.
Attachment #656673 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8595e313fc21
Target Milestone: --- → Firefox 19
Updated•12 years ago
|
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 656673 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): crash caused by bug 732336 User impact if declined: crash if you close a tab with multiple doorhanger notification messages Testing completed (on m-c, etc.): just landed on inbound Risk to taking this patch (and alternatives if risky): low-risk small self-contained change String or UUID changes made by this patch: n/a
Attachment #656673 -
Flags: approval-mozilla-beta?
Attachment #656673 -
Flags: approval-mozilla-aurora?
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8595e313fc21
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•12 years ago
|
status-firefox19:
affected → ---
Updated•12 years ago
|
Attachment #656673 -
Flags: approval-mozilla-beta?
Attachment #656673 -
Flags: approval-mozilla-beta+
Attachment #656673 -
Flags: approval-mozilla-aurora?
Attachment #656673 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 11•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/96a2697cec57 https://hg.mozilla.org/releases/mozilla-beta/rev/3a43c980820e
Comment 12•11 years ago
|
||
This is still crashing on Firefox 17.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Kevin Brosnan [:kbrosnan] from comment #12) > This is still crashing on Firefox 17. The stacks for the recent crashes look different. I looked at the most recent 10, and they don't mention DoorHangerPopup at all. Please file a new bug.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → FIXED
Comment 14•11 years ago
|
||
Verified then. I don't see any popup hanger crashes either.
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
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
•