Closed
Bug 1048984
Opened 10 years ago
Closed 10 years ago
Music app crashes frequently by asserting mOutstandingClients >0 at TextureClientPool.cpp:143
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
People
(Reporter: khuey, Assigned: milan)
References
Details
(Keywords: assertion)
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
nical
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
nical
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Clicking on songs to try to play them in the music app asserts about 1/3rd of the time for me.
Reporter | ||
Comment 1•10 years ago
|
||
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → bas
Comment 2•10 years ago
|
||
Ni :milan, since this has nominated over a month for 2.1. Not sure we would block a release on this assertion issue but could take a fix before FC.
Flags: needinfo?(milan)
Assignee | ||
Comment 3•10 years ago
|
||
If it's just in debug, I wouldn't block on it, but I would like to keep considering this and if the fix ends up being appropriate, take an uplift.
Flags: needinfo?(milan)
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #3) > If it's just in debug, I wouldn't block on it, but I would like to keep > considering this and if the fix ends up being appropriate, take an uplift. I take that back, pays to look at the code. The value is unsigned int and this assertion precedes --, so we'd end up with a very large number after this assertion and it'd probably just be a question of time before we crash somewhere.
blocking-b2g: 2.1? → 2.1+
Assignee | ||
Comment 5•10 years ago
|
||
One problem at least is calling ShrinkToMaximumSize(http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClientPool.cpp#57) after prematurely increasing mOutstandingClients http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClientPool.cpp#45
Assignee: bas → nobody
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → milan
Assignee | ||
Comment 6•10 years ago
|
||
The actual logic change is in TextureClientPool::GetTextureClient - the rest of the code are asserts and comments. I'm not sure this actually fixes the original problem, as I couldn't reproduce it myself, but it is at least a potential bug.
Attachment #8488774 -
Flags: review?(bas)
Assignee | ||
Comment 7•10 years ago
|
||
Pretty sure the above wouldn't fix the problem, given that we'd run into this assertion when mOutstandingClients is 0, but mTextureClientsDeferred size is not, I assume in a call to ReturnTextureClientDeferred(). That likely means that ReportClientLost() was called earlier, to get us out of sync in the first place. Kyle, you don't have a stack when this asserts? Just want to double check that it is ReturnTextureClientDeferred() that's the second in the stack, although I'm not sure that's very relevant, the trouble is likely caused earlier.
Flags: needinfo?(khuey)
Reporter | ||
Comment 8•10 years ago
|
||
I don't have a stack handy, and I haven't seen this in a while. I keep hitting bug 1061954 instead.
Flags: needinfo?(khuey)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #8) > I don't have a stack handy, and I haven't seen this in a while. I keep > hitting bug 1061954 instead. Thanks. I'm going to assume it's the combination of ReportClientLost and changing gfxContentType that conspire to mess up the count of outstanding tiles and post a patch that deals with it.
Assignee | ||
Updated•10 years ago
|
Attachment #8488774 -
Attachment is obsolete: true
Attachment #8488774 -
Flags: review?(bas)
Updated•10 years ago
|
Attachment #8490046 -
Flags: review?(nical.bugzilla) → review+
Comment 12•10 years ago
|
||
Comment on attachment 8490047 [details] [diff] [review] Part 2: Shrink to maximum, rather than minimum size, and keep mOutstandingClients consistent at all times. r=nsilva Review of attachment 8490047 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the added documentation!
Attachment #8490047 -
Flags: review?(nical.bugzilla) → review+
Assignee | ||
Comment 13•10 years ago
|
||
Try was run on a combined patch, otherwise only trivial changes, so I'd like to "reuse" it: https://tbpl.mozilla.org/?tree=Try&rev=b462806339e3. The patches should apply cleanly to Aurora, but let me know if there are problems.
Keywords: checkin-needed
Assignee | ||
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e6ae62618df https://hg.mozilla.org/integration/mozilla-inbound/rev/9a997d743553
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7e6ae62618df https://hg.mozilla.org/mozilla-central/rev/9a997d743553
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 16•10 years ago
|
||
Please request Aurora approval on these patches when you get a chance.
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
status-firefox33:
--- → wontfix
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
Flags: needinfo?(milan)
Assignee | ||
Updated•10 years ago
|
Attachment #8490046 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(milan)
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8490047 [details] [diff] [review] Part 2: Shrink to maximum, rather than minimum size, and keep mOutstandingClients consistent at all times. r=nsilva Approval Request Comment [Feature/regressing bug #]: 1046025 [User impact if declined]: Scenarios where we switch from transparent content to opaque, or vice versa, could cause problems with the tiling system. [Describe test coverage new/current, TBPL]: [Risks and why]: Low [String/UUID change made/needed]:
Attachment #8490047 -
Flags: approval-mozilla-aurora?
Comment 19•10 years ago
|
||
Comment on attachment 8490046 [details] [diff] [review] Part 1: Be consistent to where we report tiles lost. r=nsilva Aurora+
Attachment #8490046 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•10 years ago
|
Attachment #8490047 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 20•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/51f29a050e63 https://hg.mozilla.org/releases/mozilla-aurora/rev/ba00e8f04fa6
Comment 21•10 years ago
|
||
This bug is verified fixed in Flame 2.2 KK and 2.1 KK. Tested with Fullflash and 319mb. No assert occurs and the Music app does not crash out when playing many different songs while accessing them from different screens within the Music app. Reading the logcat also shows no signs of asserts. Device: Flame 2.2 KK BuildID: 20141013040202 Gaia: 3b81896f04a02697e615fa5390086bd5ecfed84f Gecko: f547cf19d104 Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 35.0a1 (2.2) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 ----------------------------------------------------------------- Device: Flame 2.1 KK BuildID: 20141013001201 Gaia: d18e130216cd3960cd327179364d9f71e42debda Gecko: 610ee0e6a776 Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•