Open Bug 680311 Opened 8 years ago Updated 6 months ago

Investigate android sync message queues for deadlock conditions

Categories

(Core :: Widget: Android, defect, P5)

ARM
Android
defect

Tracking

()

Tracking Status
fennec + ---

People

(Reporter: dougt, Unassigned)

References

Details

(Keywords: hang, mobile)

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → doug.turner
Attached patch patch (obsolete) — Splinter Review
Assignee: doug.turner → blassey.bugs
Attachment #555016 - Flags: review?(doug.turner)
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.
Attachment #555016 - Flags: review?(doug.turner) → review-
Attached patch patchSplinter Review
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
Attachment #555016 - Attachment is obsolete: true
Attachment #562843 - Flags: review?(doug.turner)
Comment on attachment 562843 [details] [diff] [review]
patch

so, for the file input case, we really don't want to wait 1 second first.
Attachment #562843 - Flags: review?(doug.turner) → review?(mark.finkle)
Comment on attachment 562843 [details] [diff] [review]
patch

I am not tall enough to review this code
Attachment #562843 - Flags: review?(mark.finkle) → review?(doug.turner)
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.
Attachment #562843 - Flags: review?(doug.turner) → review+
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..
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.
yes, still relevant to native fennec
filter on [mass-p5]
Priority: -- → P5
Assignee: lassey → nobody
You need to log in before you can comment on or make changes to this bug.