Closed Bug 1000103 Opened 12 years ago Closed 11 years ago

Intermittent base.py | application crashed [@ mozilla::layers::TextureImageTextureSourceOGL::GetFormat() const]

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33
Tracking Flags:
Tracking Status
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- fixed
firefox-esr24 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.3T --- unaffected
b2g-v1.4 --- wontfix
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: cbook, Assigned: nical)

References

(
URL
)

Details

(Keywords: crash, intermittent-failure, Whiteboard: [qa-] )

Attachments

(1 file, 1 obsolete file)

(v2) Reset the IncrementalContentClient if allocating a surface descriptor fails
4.01 KB, patch
mattwoodrow
: review+
lmandel
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review
b2g_macosx64 mozilla-inbound opt test gaia-ui-test on 2014-04-23 03:37:20 PDT for push 8d27ef04d88d slave: talos-mtnlion-r5-015 https://tbpl.mozilla.org/php/getParsedLog.php?id=38316118&tree=Mozilla-Inbound PROCESS-CRASH | base.py | application crashed [@ mozilla::layers::TextureImageTextureSourceOGL::GetFormat() const] 03:48:47 INFO - Operating system: Mac OS X 03:48:47 INFO - 10.8.0 12A269 03:48:47 INFO - CPU: amd64 03:48:47 INFO - family 6 model 42 stepping 7 03:48:47 INFO - 8 CPUs 03:48:47 INFO - Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS 03:48:47 INFO - Crash address: 0x0 03:48:47 INFO - Thread 21 (crashed) 03:48:47 INFO - 0 XUL!mozilla::layers::TextureImageTextureSourceOGL::GetFormat() const [TextureHostOGL.cpp:8d27ef04d88d : 320 + 0x0] 03:48:47 INFO - rbx = 0x0000000120bae580 r12 = 0x0000000000000140 03:48:47 INFO - r13 = 0x0000000000000000 r14 = 0x0000000000000000 03:48:47 INFO - r15 = 0x0000000000000000 rip = 0x00000001019e5214 03:48:47 INFO - rsp = 0x000000010a185ba8 rbp = 0x0000000120beece0 03:48:47 INFO - Found by: given as instruction pointer in context 03:48:47 INFO - 1 XUL!mozilla::layers::ContentHostIncremental::TextureCreationRequest::Execute(mozilla::layers::ContentHostIncremental*) [ContentHost.cpp:8d27ef04d88d : 569 + 0x5] 03:48:47 INFO - rbx = 0x0000000120bae580 r12 = 0x0000000000000140 03:48:47 INFO - r13 = 0x0000000000000000 r14 = 0x0000000000000000 03:48:47 INFO - r15 = 0x0000000000000000 rip = 0x00000001019be737 03:48:47 INFO - rsp = 0x000000010a185bb0 rbp = 0x0000000120beece0 03:48:47 INFO - Found by: call frame info 03:48:47 INFO - 2 XUL!mozilla::layers::ContentHostIncremental::ProcessTextureUpdates() [ContentHost.cpp:8d27ef04d88d : 457 + 0x8] 03:48:47 INFO - rbx = 0x0000000000000001 r12 = 0x0000000000000003 03:48:47 INFO - r13 = 0x0000000107030bc0 r14 = 0x0000000120beed48 03:48:47 INFO - r15 = 0x0000000120beece0 rip = 0x00000001019be120 03:48:47 INFO - rsp = 0x000000010a185cd0 rbp = 0x0000000000000000 03:48:47 INFO - Found by: call frame info 03:48:47 INFO - 3 XUL!mozilla::layers::ContentHostIncremental::Lock() [ContentHost.h:8d27ef04d88d : 283 + 0x4] 03:48:47 INFO - rbx = 0x0000000120beece0 r12 = 0x0000000120beece0 03:48:47 INFO - r13 = 0x0000000108d3d800 r14 = 0x000000010a185f87 03:48:47 INFO - r15 = 0x0000000000000000 rip = 0x00000001019bf019 03:48:47 INFO - rsp = 0x000000010a185d00 rbp = 0x0000000000000000 03:48:47 INFO - Found by: call frame info 03:48:47 INFO - 4 XUL!mozilla::layers::ContentHostBase::Composite(mozilla::layers::EffectChain&, float, mozilla::gfx::Matrix4x4 const&, mozilla::gfx::Filter const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> const&, nsIntRegion const*, mozilla::layers::TiledLayerProperties*) [ContentHost.cpp:8d27ef04d88d : 43 + 0x8] 03:48:47 INFO - rbx = 0x0000000120beece0 r12 = 0x0000000120beece0 03:48:47 INFO - r13 = 0x0000000108d3d800 r14 = 0x000000010a185f87 03:48:47 INFO - r15 = 0x0000000000000000 rip = 0x00000001019bc9a6 03:48:47 INFO - rsp = 0x000000010a185d10 rbp = 0x0000000000000000 03:48:47 INFO - Found by: call frame info
Assignee: nobody → nical.bugzilla
We are in IncrementalContentHost, trying to get the format of a TextureImageTextureSource that seems to not have been allocated (its mTexImage member is null). I see two issues: 1) TextureImageTextureSourceOGL doesn't crash in GetSize if not allocated, but does crash in GetFormat. We should have a consistent behavior there, but that won't solve the problem 2) 2) the incremental ContentHost expects its source to be allocated when processing a texture creation request with the COPY_PREIOUS flag (basically it assumes it has a previous which appears not to be the case when the crash happens). So far the only way I found to get into this state is if we have a creation request without COPY_PREVIOUS and another creation request this time with COPY_PREVIOUS, without an update request in between.
Looks like the situation I described above could be triggered by ContentClientIncremental::GetUpdateSurface failing to allocate a SurfaceDescriptor, leaving us with a surface that won't have its update. This isn't the first time I see us falling into the trap of creating objects in an incomplete state, initializing them afterwards (in a different function) and not properly check/account for the object potentially not being fully initialized. I regret introducing that in TextureClient and TextureSource.
Also removes some dead code (DestroyTextures) and makes TextureImageTextureSourceOGL observe the same behavior in GetFormat as in GetSize (warn and return a default value instead of crashing) for consistency. try push: https://tbpl.mozilla.org/?tree=Try&rev=f971593c9470
Attachment #8435774 - Flags: review?(matt.woodrow)
One of the assertion I added was wrong, new try push: https://tbpl.mozilla.org/?tree=Try&rev=ffc6e7205f6b
Attachment #8435774 - Attachment is obsolete: true
Attachment #8435774 - Flags: review?(matt.woodrow)
Attachment #8435934 - Flags: review?(matt.woodrow)
Comment on attachment 8435934 [details] [diff] [review] (v2) Reset the IncrementalContentClient if allocating a surface descriptor fails Review of attachment 8435934 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/client/ContentClient.cpp @@ +921,5 @@ > if (!mForwarder->AllocSurfaceDescriptor(rgnSize.Size().ToIntSize(), > mContentType, > &desc)) { > NS_WARNING("creating SurfaceDescriptor failed!"); > + Clear(); I don't think this will do anything interesting. ::: gfx/layers/composite/ContentHost.cpp @@ +493,5 @@ > } > > if (mTextureInfo.mDeprecatedTextureHostFlags & DeprecatedTextureHostFlags::COPY_PREVIOUS) { > + MOZ_ASSERT(aHost->mSource); > + MOZ_ASSERT(aHost->mSource->IsValid()); I assume we'll still hit this and crash here instead. If we created the texture, but the update failed (which appears to be the only way to get into this state), then we probably should just fail here too.
Comment on attachment 8435934 [details] [diff] [review] (v2) Reset the IncrementalContentClient if allocating a surface descriptor fails Review of attachment 8435934 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/client/ContentClient.cpp @@ +921,5 @@ > if (!mForwarder->AllocSurfaceDescriptor(rgnSize.Size().ToIntSize(), > mContentType, > &desc)) { > NS_WARNING("creating SurfaceDescriptor failed!"); > + Clear(); Never mind, it'll stop us sending COPY_PREVIOUS next time.
Attachment #8435934 - Flags: review?(matt.woodrow) → review+
This seems to have gone away with nical's patch. But now we're hitting bug 1022201 ~50% of the time instead.
Depends on: 1023027
https://hg.mozilla.org/mozilla-central/rev/223ebf38ddd9
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Definitely need this on Aurora for B2G 2.0. Not sure if we need it on b2g30 for B2G v1.4 and/or Beta for Fx31 as well.
status-b2g-v1.4: --- → ?
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
status-firefox31: --- → ?
status-firefox32: --- → affected
status-firefox33: --- → fixed
Flags: needinfo?(nical.bugzilla)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #119) > This seems to have gone away with nical's patch. But now we're hitting bug > 1022201 ~50% of the time instead. Looking at bug 1022201 it's really hard to connect the crash with anything gfx related. I wonder if this bug was just hiding the new one. (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #121) > Definitely need this on Aurora for B2G 2.0. Not sure if we need it on b2g30 > for B2G v1.4 and/or Beta for Fx31 as well. This is not needed for b2g since the code in question is only used on mac.
Flags: needinfo?(nical.bugzilla)
The patch applies without conflict on aurora.
Mac-only can still mean needs landing on beta31 :)
Comment on attachment 8435934 [details] [diff] [review] (v2) Reset the IncrementalContentClient if allocating a surface descriptor fails [Approval Request Comment] Bug caused by (feature/regressing bug #): no idea User impact if declined: Crashes on OSX Testing completed (on m-c, etc.): a few days Risk to taking this patch (and alternatives if risky): low, no alternative String or IDL/UUID changes made by this patch:
Attachment #8435934 - Flags: approval-mozilla-aurora?
So this is wontfix for Fx31? :)
status-b2g-v1.4: ? → wontfix
Comment on attachment 8435934 [details] [diff] [review] (v2) Reset the IncrementalContentClient if allocating a surface descriptor fails Aurora approval granted. As Ryan asked, can you confirm that this fix is not needed on beta 31?
Attachment #8435934 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
The first occurrence of the crash appearing in this bug was on 2014-04-23 and Fx31 started on 2014-03-17 (I am looking at https://wiki.mozilla.org/RapidRelease/Calendar and I am not 100% sure whether the date is when the branch starts or when it is merged into the next cycle) so I suppose we should uplift it to 31 as well.
Flags: needinfo?(nical.bugzilla)
Comment on attachment 8435934 [details] [diff] [review] (v2) Reset the IncrementalContentClient if allocating a surface descriptor fails Same answers as comment 125. No issues since it landed on trunk or Aurora.
Attachment #8435934 - Flags: approval-mozilla-beta?
Attachment #8435934 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: