Investigate android sync message queues for deadlock conditions

NEW
Assigned to

Status

()

Core
Widget: Android
P5
normal
6 years ago
3 years ago

People

(Reporter: dougt, Assigned: blassey)

Tracking

({hang, mobile})

unspecified
ARM
Android
hang, mobile
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Assignee: nobody → doug.turner
Created attachment 555016 [details] [diff] [review]
patch
Assignee: doug.turner → blassey.bugs
Attachment #555016 - Flags: review?(doug.turner)
(Reporter)

Comment 2

6 years ago
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.
(Reporter)

Updated

6 years ago
Attachment #555016 - Flags: review?(doug.turner) → review-
Created attachment 562843 [details] [diff] [review]
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
Attachment #555016 - Attachment is obsolete: true
Attachment #562843 - Flags: review?(doug.turner)
(Reporter)

Comment 4

6 years ago
Comment on attachment 562843 [details] [diff] [review]
patch

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

Updated

6 years ago
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)
(Reporter)

Comment 6

6 years ago
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+
(Reporter)

Comment 7

6 years ago
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..
Blocks: 724109
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
You need to log in before you can comment on or make changes to this bug.