680311 - Investigate android sync message queues for deadlock conditions

Status: NEW

(GeckoView :: General, defect, P5)

Description

[Doug Turner (:dougt)]

In Bug #661978, we found that under certain conditions, we can deadlock the gecko thread.  We solve this by nesting an event loop to process incoming messages.

This bug is to investigate other sync messages queues we use.

Comment 1

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

Attachment: patch

Comment 2

[Doug Turner (:dougt)]

brad and i discuss this a bit last week.  I don't like the idea of changing all of these call sites over to spinning a nested loop.  I filed the bug mostly to investigate other call sites for similar deadlocking conditions.  We should only spin a nested loop when we completely understand the events that trigger a deadlock.

He suggested that in cases we do not completely understand the event loop (every cases but the file picker dialog), the initial wait would be >= 1s.  Then we would spin a event loop.  If we did this, we should add logging to see how often this is hit and we should use that to investigate further.

Comment 3

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

Attachment: patch

this implements what dougt and I discussed. Essentially, behavior is unchanged unless we block for more than 1s, in which case we'll spin up the gecko message loop

Comment 4

[Doug Turner (:dougt)]

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

so, for the file input case, we really don't want to wait 1 second first.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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

I am not tall enough to review this code

Comment 6

[Doug Turner (:dougt)]

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

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

don't do that first 1s wait. always just loop for 1ms.  sorry for the long delay at reviewing this.

Comment 7

[Doug Turner (:dougt)]

also, keep in mind, that I am not sure why we'd need to spin any loop other than for the filepicker.  in that case we know that we have reentrancy.  doing the rest, seems to be overkill, but if it makes flash better..

Comment 8

[Kevin Brosnan [Ex-Mozilla]]

Is this patch/bug still useful for Native? I see the file still exists in hg and looks like the sections of code it modifies still exist.

Comment 9

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

yes, still relevant to native fennec

Comment 10

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

filter on [mass-p5]

Comment 11

[David Lawrence [:dkl]]

Moving all open Core::Widget: Android bugs to GeckoView::General (then the triage owner of GeckoView will decide which ones are valuable and which ones should be closed).