Open
Bug 680311
Opened 13 years ago
Updated 2 years ago
Investigate android sync message queues for deadlock conditions
Categories
(GeckoView :: General, defect, P5)
Tracking
(fennec+)
NEW
Tracking | Status | |
---|---|---|
fennec | + | --- |
People
(Reporter: dougt, Unassigned)
References
Details
(Keywords: hang, mobile)
Attachments
(1 file, 1 obsolete file)
14.15 KB,
patch
|
dougt
:
review+
|
Details | Diff | Splinter Review |
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•13 years ago
|
Assignee: nobody → doug.turner
Comment 1•13 years ago
|
||
Assignee: doug.turner → blassey.bugs
Attachment #555016 -
Flags: review?(doug.turner)
Reporter | ||
Comment 2•13 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•13 years ago
|
Attachment #555016 -
Flags: review?(doug.turner) → review-
Comment 3•13 years ago
|
||
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•13 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.
Updated•13 years ago
|
Attachment #562843 -
Flags: review?(doug.turner) → review?(mark.finkle)
Comment 5•13 years ago
|
||
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•13 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•13 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..
Comment 8•12 years ago
|
||
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•12 years ago
|
||
yes, still relevant to native fennec
Updated•5 years ago
|
Assignee: lassey → nobody
Comment 11•3 years ago
|
||
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).
Component: Widget: Android → General
Product: Core → GeckoView
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•