Closed
Bug 1213050
Opened 9 years ago
Closed 8 years ago
Intermittent test_zmedia_cleanup.html | application crashed [@ mozilla::(anonymous namespace)::RunWatchdog(void*)]
Categories
(Core :: WebRTC, defect, P1)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: KWierso, Assigned: jesup)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file)
2.71 KB,
patch
|
cpearce
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Updated•9 years ago
|
Priority: -- → P5
Updated•9 years ago
|
Component: Audio/Video → WebRTC
Priority: P5 → --
Updated•9 years ago
|
Rank: 35
Priority: -- → P3
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 5•8 years ago
|
||
Andreas - what's the reason for the 'blocks'? I see this jumped around 2 weeks ago; what's the regression source, if you have an idea? We need to find a way to get more info about why something fails shutdown....
Flags: needinfo?(pehrsons)
Comment 6•8 years ago
|
||
It blocks because of https://treeherder.mozilla.org/#/jobs?repo=try&revision=da5e30ca94c6. I have no idea of any regression source really. Cloning?
Flags: needinfo?(pehrsons)
Comment hidden (Intermittent Failures Robot) |
Comment 8•8 years ago
|
||
Maire, this is our number 3 intermittent. Can we get someone from the media team to take a look at it?
Flags: needinfo?(mreavy)
Assignee | ||
Comment 9•8 years ago
|
||
We need to clean up the TaskQueue for VideoFrameConverter *before* dropping the reference to it, since if the last reference is held by an event in the TaskQueue, it will try to clean itself up (and fail). Also adds an assertion to TaskQueue against self-immolation
Attachment #8751547 -
Flags: review?(cpearce)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff16a3031beb Nathan: I added an assert against a TaskQueue trying to swallow it's tail and kill itself. Once thing we should consider is if a TaskQueue should allow BeginShutdown/AwaitShutdownAndIdle() more than once (return a new promise that resolves when the original does or immediately, for exmaple), or if it shoudl be held to be an error (MOZ_ASSERT). I sidestepped it by coding such that I'll continue to call them once.
Flags: needinfo?(nfroyd)
Updated•8 years ago
|
Rank: 35 → 15
Flags: needinfo?(mreavy)
Priority: P3 → P1
Comment 11•8 years ago
|
||
Comment on attachment 8751547 [details] [diff] [review] clean up TaskQueue before dropping references to it Review of attachment 8751547 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/threads/TaskQueue.cpp @@ +98,5 @@ > > void > TaskQueue::AwaitShutdownAndIdle() > { > + MOZ_ASSERT(!IsCurrentThreadIn()); Fingers crossed this doesn't turn the tree red! Maybe it'll make bug 1264694 permafail.
Attachment #8751547 -
Flags: review?(cpearce) → review+
Comment 13•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/52abfa1b6229
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment hidden (Intermittent Failures Robot) |
Comment 16•8 years ago
|
||
Please request Aurora approval on this when you get a chance. I don't see any obvious hits of this on Beta, but feel free to nominate there too if you feel it's warranted.
status-firefox48:
--- → affected
Flags: needinfo?(rjesup)
Assignee | ||
Comment 17•8 years ago
|
||
Comment on attachment 8751547 [details] [diff] [review] clean up TaskQueue before dropping references to it Approval Request Comment [Feature/regressing bug #]: 1208371 (only in 48 and newer) [User impact if declined]: Random shutdown hangs, mochitest intermittent [Describe test coverage new/current, TreeHerder]: It shows up as an intermittent [Risks and why]: Very low risk; mostly moves where BeginShutdown/etc is called to avoid it happening on the TaskQueue thread itself (and added assertion against this happening, here or elsewhere). [String/UUID change made/needed]: none
Attachment #8751547 -
Flags: approval-mozilla-aurora?
Comment 18•8 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #10) > Once thing we should consider is if a TaskQueue should allow > BeginShutdown/AwaitShutdownAndIdle() more than once (return a new promise > that resolves when the original does or immediately, for exmaple), or if it > shoudl be held to be an error (MOZ_ASSERT). I sidestepped it by coding such > that I'll continue to call them once. I assume this is the reason for the needinfo. I think you could reasonably do it either way. Our threads and thread pool implementations permit Shutdown to be called on them multiple times (though nsThread will complain at you in a debug build). Trying to enforce a single call to Shutdown will help make our shutdown code more sensible, I think, so let's go with that if possible.
Flags: needinfo?(nfroyd)
Comment 19•8 years ago
|
||
Comment on attachment 8751547 [details] [diff] [review] clean up TaskQueue before dropping references to it Fix an intermittent, taking it
Attachment #8751547 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 20•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/b8dac4b0e8e0
You need to log in
before you can comment on or make changes to this bug.
Description
•