Bug 8829

Created attachment 7664 [details] [review]
add null check

Untested patch; sorry, I don't have git, so I just stole a copy of the file off
gitweb...

Diff is against whatever
http://gitweb.freedesktop.org/?p=cairo.git;a=blob_plain;h=7aeb2566f3b503bfda76603e35f60c30655176c4;f=src/cairo-win32-surface.c
is (10-28 ish?)

Comment on attachment 7664 [details] [review]
add null check

I am an idiot :p  Trying to write a patch that actually makes sense this time,
sorry.

Created attachment 7665 [details] [review]
surface status check instead

Do  you have any idea why the call to surface_create() is failing? The
patch in itself looks fine to me, but I think if you actually hit that
problem, there is some other bug going on.

no, but i'm not the only person to hit it. someone else filed a
bugzilla.mozilla.org bug complaining about it.

for some reason i didn't save a .dmp from it. We have symbols for everything, so
if you're ok w/ donwloading say a 50mb zip of a 250mb .dmp plus symbols (60mb or
so), i can try to remember to save a dump if i hit it again. in theory the bug
filer might have included a testcase or steps to reproduce, but i didn't pay
attn to the bug, just recognized that it matched my stack and this bug which i
had already filed.

no guarantees, i don't actually crash that often, and i certainly don't go
looking for specific crashes.

The relevant Mozilla bugs are
https://bugzilla.mozilla.org/show_bug.cgi?id=348644 and
https://bugzilla.mozilla.org/show_bug.cgi?id=359589
I added a backtrace in bug 348644, maybe useful?

not sure what you mean by failing

cairo_win32_surface_create: The handle is invalid.

0:000> dt new_surf base.status
Local var @ 0x12e944 Type _cairo_win32_surface*
0x01b94738 
   +0x000 base        : 
      +0x010 status      : 1 ( CAIRO_STATUS_NO_MEMORY )

my gecko is using 1.75g of vm and normally will crash around 1.9g for oom.

offhand, i'd guess it's failing because we're calling it w/ a null pointer....

thebes!_cairo_win32_surface_create_similar_internal(void * abstract_src = 0x73ca5e88, _cairo_content content = CAIRO_CONTENT_COLOR (4096), int width = 579, int height = 317, int force_dib = 0)+0xde [cairo\cairo\src\cairo-win32-surface.c @ 418]
thebes!_cairo_win32_surface_create_similar(void * abstract_src = 0x73ca5e88, _cairo_content content = CAIRO_CONTENT_COLOR (4096), int width = 579, int height = 317)+0x1a [cairo\cairo\src\cairo-win32-surface.c @ 432]
thebes!_cairo_surface_create_similar_scratch(struct _cairo_surface * other = 0x73ca5e88, _cairo_content content = CAIRO_CONTENT_COLOR (4096), int width = 579, int height = 317)+0x54 [cairo\cairo\src\cairo-surface.c @ 285]
thebes!_cairo_surface_create_similar_solid(struct _cairo_surface * other = 0x73ca5e88, _cairo_content content = CAIRO_CONTENT_COLOR (4096), int width = 579, int height = 317, struct _cairo_color * color = 0x01b94ed0)+0x1b [cairo\cairo\src\cairo-surface.c @ 357]
thebes!_moz_cairo_surface_create_similar(struct _cairo_surface * other = 0x73ca5e88, _cairo_content content = CAIRO_CONTENT_COLOR (4096), int width = 579, int height = 317)+0x55 [cairo\cairo\src\cairo-surface.c @ 341]
thebes!_moz_cairo_push_group_with_content(struct _cairo * cr = 0x73ca5fb0, _cairo_content content = CAIRO_CONTENT_COLOR (4096))+0x76 [cairo\cairo\src\cairo.c @ 424]
thebes!gfxContext::PushGroup(gfxASurface::gfxContentType content = CONTENT_COLOR (4096))+0x17 [thebes\src\gfxcontext.cpp @ 662]
gkwidget!nsWindow::OnPaint(struct HDC__ * aDC = 0x00000000)+0x436 

I can't imagine GetClipRect doing something remotely useful for a null pointer...

0:000> dv
            ddb = 0x0c0567ac
           crgn = 0x170456f1
         ddb_dc = 0x00000000
saved_dc_bitmap = 0x00000000
   abstract_src = 0x73ca5e88
        content = CAIRO_CONTENT_COLOR (4096)
          width = 579
         height = 317
      force_dib = 0
            src = 0x73ca5e88
         format = CAIRO_FORMAT_RGB24 (1)
       new_surf = 0x01b94738

The call is:
	new_surf = (cairo_win32_surface_t*) cairo_win32_surface_create (ddb_dc);

The logic checks:
0:000> dt force_dib
Local var @ 0x12e960 Type int
0
0:000> dt src is_dib
Local var @ 0x12e93c Type _cairo_win32_surface*
0x73ca5e88 
   +0x0d4 is_dib : 0
0:000> dt content
Local var @ 0x12e954 Type _cairo_content
1000 ( CAIRO_CONTENT_COLOR )

0:000> dt src dc
Local var @ 0x12e93c Type _cairo_win32_surface*
0x73ca5e88 
   +0x000 base             : _cairo_surface
   +0x0c8 format           : 1 ( CAIRO_FORMAT_RGB24 )
   +0x0cc dc               : 0xbd0154d0 HDC__
   +0x0d0 bitmap           : (null) 
   +0x0d4 is_dib           : 0
   +0x0d8 saved_dc_bitmap  : (null) 
   +0x0dc image            : (null) 
   +0x0e0 clip_rect        : _cairo_rectangle_int16
   +0x0e8 saved_clip       : (null) 
   +0x0ec extents          : _cairo_rectangle_int16
   +0x0f4 flags            : 0x3e
0:000> dt width
Local var @ 0x12e958 Type int
579
0:000> dt height
Local var @ 0x12e95c Type int
317

Unfortunately i keep hanging my debugger trying to play with this.

If my debugger survives long enough, i'll try to use GetLastError and see if something interesting shows up.

Reading through the mozilla bugs, the suggested cause was hitting a limit on the number of GDI objects per-process causing the handle creation routing to fail. With the cause understood and the status checked, we can close this bug.