Last Comment Bug 921215 - crash in MacIOSurface::GetWidth()
: crash in MacIOSurface::GetWidth()
Status: RESOLVED FIXED
[qa-]
: crash, hang
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All macOS
-- critical (vote)
: mozilla28
Assigned To: Dan Glastonbury (:kamidphish) | needinfo me
:
: Jessie [:jbonisteel] plz needinfo
Mentors:
http://www.unrealengine.com/html5/
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-26 14:40 PDT by Martijn Wargers (zombie)
Modified: 2013-12-11 16:17 PST (History)
8 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
-
affected
affected
affected


Attachments
log.txt (382.24 KB, text/plain)
2013-09-26 14:55 PDT, Martijn Wargers (zombie)
no flags Details
Handle being passed NULL ptr for either surface or gl to SharedSurface_IOSurface::Create by returning NULL. (1.14 KB, patch)
2013-12-02 16:32 PST, Dan Glastonbury (:kamidphish) | needinfo me
no flags Details | Diff | Splinter Review
Handle being passed NULL ptr for either surface or gl toSharedSurface_IOSurface::Create by returning NULL. (1.41 KB, patch)
2013-12-02 19:15 PST, Dan Glastonbury (:kamidphish) | needinfo me
mattwoodrow: review+
Details | Diff | Splinter Review

Description User image Martijn Wargers (zombie) 2013-09-26 14:40:47 PDT
I got this crash on my Macbook Pro, using MacOsX 10.7.5, using Firefox 25.
I could not reproduce this crash, I only got it the first time I visited the site. Afterwards, I get hangs/freezes on that site.

What I did:
1. Load http://www.unrealengine.com/html5/ in a new tab
2. Click the 'Play' button and wait for the data to download (might take a couple minutes)
3. If you get a warning about storing offline data, click 'Allow' and wait a couple more minutes for some more loading
4. Go to full screen (with the Apple full screen button on the top right).

This bug was filed from the Socorro interface and is 
report bp-7d59f361-fe46-47b7-ad9e-856e82130926.
=============================================================
0 	XUL 	MacIOSurface::GetWidth() 	gfx/2d/QuartzSupport.mm
1 	XUL 	mozilla::gl::SurfaceFactory_IOSurface::CreateShared(nsIntSize const&) 	gfx/gl/SharedSurfaceIO.cpp
2 	XUL 	mozilla::gfx::SurfaceStream_TripleBuffer::SwapProducer(mozilla::gfx::SurfaceFactory*, nsIntSize const&) 	gfx/gl/SurfaceStream.cpp
3 	XUL 	mozilla::gl::GLScreenBuffer::PublishFrame(nsIntSize const&) 	gfx/gl/GLScreenBuffer.cpp
4 	XUL 	mozilla::gl::GLContext::PublishFrame() 	gfx/gl/GLContext.cpp
5 	XUL 	mozilla::WebGLContext::PresentScreenBuffer() 	content/canvas/src/WebGLContext.cpp
6 	XUL 	mozilla::WebGLContextUserData::PreTransactionCallback(void*) 	content/canvas/src/WebGLContext.cpp
7 	XUL 	mozilla::layers::ClientCanvasLayer::RenderLayer() 	gfx/layers/Layers.h
8 	XUL 	ClientContainerLayer::RenderLayer() 	gfx/layers/client/ClientContainerLayer.h
9 	XUL 	ClientContainerLayer::RenderLayer() 	gfx/layers/client/ClientContainerLayer.h
10 	XUL 	mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/client/ClientLayerManager.cpp
etc...
Comment 1 User image Anthony Hughes (:ashughes) [QA] 2013-09-26 14:51:50 PDT
Thanks Martijn, can you try to regress the hang?
Comment 2 User image Martijn Wargers (zombie) 2013-09-26 14:55:36 PDT
Created attachment 810787 [details]
log.txt

This is a log of the hang.
I haven't been able to reproduce the hang in a clean profile yet, so this might be extension related or something like that.
Comment 3 User image Martijn Wargers (zombie) 2013-09-26 15:21:48 PDT
Ok, I managed to get the hang also in a clean profile in Firefox25.
I haven't been able to get the hang (yet?) in Firefox24.
Comment 4 User image Henrik Skupin (:whimboo) [⌚️UTC+1] 2013-09-27 04:34:10 PDT
Martijn, what about aurora and nightly? Do they behave the same?
Comment 5 User image Alex Keybl [:akeybl] 2013-09-30 05:01:27 PDT
Over to Milan to help with investigation, and including mbest so that he's in the loop on this Unreal Engine related regression.
Comment 6 User image Kyle Wooten 2013-10-03 19:08:52 PDT
Thanks!
Comment 7 User image Anthony Hughes (:ashughes) [QA] 2013-10-08 10:52:57 PDT
Martijn, can you please investigate the regression window further using the Firefox 25 Nightly builds?
Comment 8 User image Martijn Wargers (zombie) 2013-10-08 11:44:44 PDT
I can try and find a regression window for the hang, the crash seemed to be a one time only event.
Comment 9 User image Martijn Wargers (zombie) 2013-10-09 10:06:57 PDT
I'm afraid, I can't seem to reproduce this hang anymore in any build, whatsoever (tried Firefox 24, 25 and trunk).
Comment 10 User image Alex Keybl [:akeybl] 2013-10-09 12:32:07 PDT
WFM in that case. Thanks Martijn!
Comment 11 User image Martijn Wargers (zombie) 2013-10-09 13:07:09 PDT
Well, I haven't been able to reproduce, but it seems to me, it should be still there.
Comment 12 User image Henrik Skupin (:whimboo) [⌚️UTC+1] 2013-10-09 13:23:26 PDT
Absolutely. I can see crash reports with a yesterdays build, which makes it hard for me to believe that this crash has been fixed as of today.

See https://crash-stats.mozilla.com/report/index/6ee5db6f-00ec-420d-b53a-7c2d32131009
Comment 13 User image Anthony Hughes (:ashughes) [QA] 2013-10-10 13:43:56 PDT
More reports:
https://crash-stats.mozilla.com/query/?query_type=simple&query=MacIOSurface%3A%3AGetWidth%28%29

While this does have more reports in Firefox 25, 26, and 27. At 12 reports in the last week across all versions this is well below our threshold for tracking and assigning investigative resources at this time.
Comment 14 User image Dan Glastonbury (:kamidphish) | needinfo me 2013-12-02 16:23:14 PST
In response to the crash reports in Comment 13, a quick code review suggests that in:

/* static */ SharedSurface_IOSurface*
SharedSurface_IOSurface::Create(MacIOSurface* surface, GLContext *gl, bool hasAlpha)
{
  gfxIntSize size(surface->GetWidth(), surface->GetHeight());

surface is NULL. I'll add handling code for this case.
Comment 15 User image Dan Glastonbury (:kamidphish) | needinfo me 2013-12-02 16:32:27 PST
Created attachment 8341401 [details] [diff] [review]
Handle being passed NULL ptr for either surface or gl to SharedSurface_IOSurface::Create by returning NULL.

Matt, Does this look right?
Comment 16 User image Dan Glastonbury (:kamidphish) | needinfo me 2013-12-02 16:34:57 PST
https://tbpl.mozilla.org/?tree=Try&rev=45acaf889050
Comment 17 User image Matt Woodrow (:mattwoodrow) 2013-12-02 17:26:59 PST
Comment on attachment 8341401 [details] [diff] [review]
Handle being passed NULL ptr for either surface or gl to SharedSurface_IOSurface::Create by returning NULL.

Review of attachment 8341401 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/SharedSurfaceIO.cpp
@@ +16,5 @@
>  
>  /* static */ SharedSurface_IOSurface*
>  SharedSurface_IOSurface::Create(MacIOSurface* surface, GLContext *gl, bool hasAlpha)
>  {
> +    if (!surface) {

Might as well move this into the caller (at the bottom of this file), since that's where the failure to create the MacIOSurface happens.

@@ +21,5 @@
> +        NS_WARNING("SharedSurface_IOSurface::Create: surface passed NULL.");
> +        return nullptr;
> +    }
> +
> +    if (!gl) {

Does this ever happen? I think this should be a hard assert.
Comment 18 User image Dan Glastonbury (:kamidphish) | needinfo me 2013-12-02 17:59:24 PST
(In reply to Matt Woodrow (:mattwoodrow) from comment #17)
> Comment on attachment 8341401 [details] [diff] [review]
> @@ +21,5 @@
> > +        NS_WARNING("SharedSurface_IOSurface::Create: surface passed NULL.");
> > +        return nullptr;
> > +    }
> > +
> > +    if (!gl) {
> 
> Does this ever happen? I think this should be a hard assert.

I added it just for completeness.
Comment 19 User image Dan Glastonbury (:kamidphish) | needinfo me 2013-12-02 19:15:04 PST
Created attachment 8341479 [details] [diff] [review]
Handle being passed NULL ptr for either surface or gl toSharedSurface_IOSurface::Create by returning NULL.

Updated to follow review comments.
Comment 20 User image Dan Glastonbury (:kamidphish) | needinfo me 2013-12-02 19:25:13 PST
https://tbpl.mozilla.org/?tree=Try&rev=591830a941e2
Comment 21 User image Ryan VanderMeulen [:RyanVM] 2013-12-03 05:56:46 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/1552300c12e3
Comment 22 User image Ryan VanderMeulen [:RyanVM] 2013-12-03 13:51:31 PST
https://hg.mozilla.org/mozilla-central/rev/1552300c12e3
Comment 23 User image Anthony Hughes (:ashughes) [QA] 2013-12-10 16:07:47 PST
Martijn, can you please see if this crash reproduces for you in the latest Nightly?
Comment 24 User image Martijn Wargers (zombie) 2013-12-11 06:54:45 PST
Sorry, I can't confirm this bug to be fixed or not, this crash was a one time only crash for me.

Note You need to log in before you can comment on or make changes to this bug.