621484 - Hang at org.mozilla.gecko.GeckoInputConnection.getExtractedText(GeckoInputConnection.java:252)

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

I've been seeing this hang lately. It seems to be particularly exacerbated on startup by the patch on bug 619626, so it seems like some sort of race condition. One obvious fix to avoid this situation is to use poll() with a timeout rather than take(), this patch does that.

Comment 1

[Michael Wu [:mwu]]

Comment on attachment 499801 [details] [diff] [review]
patch

Lets not work around locking issues without understanding them. This issue suggests that either the gecko thread is hanging or there is code not honoring the requirement to respond to all queries. I'd tend towards the gecko thread hanging since I haven't seen any changes to the code that would cause the other issue.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch for check sendEventToGecko ret val solution

I've confirmed this is a race condition. The problem is that the gecko event loop isn't running yet, so sendeventtogecko fails silently.

There are two solutions, the first is to return something from sendEventToGecko indicating the event wasn't sent and bail. The other is to queue up  the events and send them through when the event loop is ready (like attachment 485307 [details] [diff] [review] on bug 601282 does). This patch implements the former solution. Its not clear how far we should take this, but it might make sense to do it anywhere where we're blocking on a response from the gecko event loop (I think there's ~10 of those places in the code)

I also wonder if bug 621062 has a similar cause.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch for queue events solution

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 499991 [details] [diff] [review]
patch for queue events solution

If we go with this approach we probably want to filter out touch and key events. Everything else should be valid.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 499991 [details] [diff] [review]
patch for queue events solution

looks good to me.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

2.0b4+ because this is blocking a blocker and it prevents a hang

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/b016aa1878fa