ANR: GeckoAppShell.pumpMessageLoop() can block the Gecko thread

RESOLVED FIXED in Firefox 25

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

Trunk
Firefox 25
All
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ANR])

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
GeckoAppShell.pumpMessageLoop() calls GeckoAppShell.getNextMessageFromQueue() through JNI to retrieve the next message from the Gecko thread message queue. Because we don't want MessageQueue.next() to block, getNextMessageFromQueue() bails if |MessageQueue.mMessages == null|. However that's not a reliable way to detect whether MessageQueue.next() will block or not, and it can still block if there are pending messages (e.g. delayed messages). Once MessageQueue.next() blocks, the Gecko thread is blocked, causing unresponsiveness and ANRs.
(Assignee)

Comment 1

5 years ago
Created attachment 770842 [details] [diff] [review]
Refactor and add a Gecko handler to ThreadUtils (v1)

We first need a Gecko handler in order for the fix to work. This patch also moves setting the Gecko thread/handler to inside GeckoThread.
Attachment #770842 - Flags: review?(blassey.bugs)
(Assignee)

Comment 2

5 years ago
Created attachment 770844 [details] [diff] [review]
Use IdleHandler to make MessageQueue.next() not block (v1)

The fix here uses an IdleHandler to detect when MessageQueue.next() is going to block or not, instead of checking MessageQueue.mMessages. The IdleHandler cannot throw an Exception to prevent blocking, but it can queue a new, special message. This new message gets returned in MessageQueue.next() and we return early when we detect this message (this patch uses Message.obj to set/detect the special message).
Attachment #770844 - Flags: review?(blassey.bugs)
Attachment #770842 - Flags: review?(blassey.bugs) → review+
Comment on attachment 770844 [details] [diff] [review]
Use IdleHandler to make MessageQueue.next() not block (v1)

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

Not going to lie, there is some black magic in this code. Let's keep an eye out for flash bugs after this lands.
Attachment #770844 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/66427620c3cd
https://hg.mozilla.org/mozilla-central/rev/487b02e3c084
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.