Bug 62885

Summary: [Patch] Fix for crasher in quartz backend
Product: cairo Reporter: Michael Hutchinson <m.j.hutchinson>
Component: quartz backendAssignee: Vladimir Vukicevic <vladimir>
Status: RESOLVED FIXED QA Contact: cairo-bugs mailing list <cairo-bugs>
Severity: normal    
Priority: medium CC: mitch
Version: 1.12.14   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Fixes the crash.

Description Michael Hutchinson 2013-03-28 23:50:49 UTC
Created attachment 77176 [details]
Fixes the crash.

There is currently a crash in the Quartz backend when performing certain operations. it's relatively easy to trigger, and actually happens 7 times in the test suite. It appears to have been a known problem for almost a year: http://lists.freedesktop.org/archives/cairo/2012-April/022950.html

When the problem is triggered, CoreGraphics prints several error messages similar to the following:
context_reclaim: invalid context 0x1da71050
context_finalize: invalid context 0x1da71050

Then there is a hard crash:
malloc: *** error for object 0x24485004: incorrect
checksum for freed object - object was probably modified after being freed.
[snip]
    4   libsystem_c.dylib                   0x9889b4ec abort + 168
    5   libsystem_c.dylib                   0x98885227 szone_error + 443
    6   libsystem_c.dylib                   0x98886482 free_list_checksum_botch + 50
    7   libsystem_c.dylib                   0x98886564 tiny_free_list_remove_ptr + 112
    8   libsystem_c.dylib                   0x9887ff11 szone_free + 993
    9   CoreFoundation                      0x954cad38 __CFAllocatorSystemDeallocate + 24
    10  CoreFoundation                      0x954cad18 CFAllocatorDeallocate + 232
    11  CoreFoundation                      0x954caa7a CFRelease + 2042
    12  libcairo.2.dylib                    0x06615d25 _cairo_quartz_teardown_state + 117
    13  libcairo.2.dylib                    0x06616fba _cairo_quartz_cg_fill + 378
    14  libcairo.2.dylib                    0x0656318b _cairo_compositor_fill + 251
    15  libcairo.2.dylib                    0x06618375 _cairo_quartz_surface_fill + 165
    16  libcairo.2.dylib                    0x065d42eb _cairo_surface_fill + 315
    17  libcairo.2.dylib                    0x0656fbdc _cairo_gstate_fill + 812
    18  libcairo.2.dylib                    0x06567e8c _cairo_default_context_fill + 44
    19  libcairo.2.dylib                    0x0655a69b cairo_fill + 43


I'm pretty sure the problem is that _cairo_quartz_teardown_state is releasing something it doesn't own. It does:
CGContextRelease (state->cgDrawContext);

But, the only places that this field is assigned, it comes from a *Get* function and is therefore not owned by the caller, and is not ref'fed either:
state->cgDrawContext = CGLayerGetContext (state->layer);

Since the state also keeps a reference to the layer that owns the context, ref'ing it would be redundant. Instead, I just removed the superfluous CGContextRelease.

The attaches patch fixes the 7 crashes in the test suite, and the crashes in the application I'm working on.
Comment 1 Uli Schlachter 2013-03-29 11:40:19 UTC
Benjamin pushed this patch as commit fdec6b37596d8b064ff082326d7189daa8208052.
Comment 2 Andrea Canciani 2013-03-29 14:26:30 UTC
(In reply to comment #0)
> Created attachment 77176 [details]
> Fixes the crash.
> 
> There is currently a crash in the Quartz backend when performing certain
> operations. it's relatively easy to trigger, and actually happens 7 times in
> the test suite. It appears to have been a known problem for almost a year:
> http://lists.freedesktop.org/archives/cairo/2012-April/022950.html

Actually I still cannot reproduce the issue (fun fact: I don't get the crash even if I release the cgDrawContext multiple times, so there might be some kind of auto-ignore in the libs I'm using).
I'm running Lion (10.7.5) with an up-to-date development environment.
What environment should I use to reproduce the crashes on 9e0748e223cfb8c5557c73f3ab5068ec1323e7c2 ?

Side note: the CGContextRelease(status->cgDrawContext) is hit in 21 tests, yet only 7 were crashing, so there might be something more to be fixed.

> 
> When the problem is triggered, CoreGraphics prints several error messages
> similar to the following:
> context_reclaim: invalid context 0x1da71050
> context_finalize: invalid context 0x1da71050
> 
> Then there is a hard crash:
> malloc: *** error for object 0x24485004: incorrect
> checksum for freed object - object was probably modified after being freed.
> [snip]
>     4   libsystem_c.dylib                   0x9889b4ec abort + 168
>     5   libsystem_c.dylib                   0x98885227 szone_error + 443
>     6   libsystem_c.dylib                   0x98886482
> free_list_checksum_botch + 50
>     7   libsystem_c.dylib                   0x98886564
> tiny_free_list_remove_ptr + 112
>     8   libsystem_c.dylib                   0x9887ff11 szone_free + 993
>     9   CoreFoundation                      0x954cad38
> __CFAllocatorSystemDeallocate + 24
>     10  CoreFoundation                      0x954cad18 CFAllocatorDeallocate
> + 232
>     11  CoreFoundation                      0x954caa7a CFRelease + 2042
>     12  libcairo.2.dylib                    0x06615d25
> _cairo_quartz_teardown_state + 117
>     13  libcairo.2.dylib                    0x06616fba _cairo_quartz_cg_fill
> + 378
>     14  libcairo.2.dylib                    0x0656318b
> _cairo_compositor_fill + 251
>     15  libcairo.2.dylib                    0x06618375
> _cairo_quartz_surface_fill + 165
>     16  libcairo.2.dylib                    0x065d42eb _cairo_surface_fill +
> 315
>     17  libcairo.2.dylib                    0x0656fbdc _cairo_gstate_fill +
> 812
>     18  libcairo.2.dylib                    0x06567e8c
> _cairo_default_context_fill + 44
>     19  libcairo.2.dylib                    0x0655a69b cairo_fill + 43
> 
> 
> I'm pretty sure the problem is that _cairo_quartz_teardown_state is
> releasing something it doesn't own. It does:
> CGContextRelease (state->cgDrawContext);
> 
> But, the only places that this field is assigned, it comes from a *Get*
> function and is therefore not owned by the caller, and is not ref'fed either:
> state->cgDrawContext = CGLayerGetContext (state->layer);

Sorry, my fault.

> 
> Since the state also keeps a reference to the layer that owns the context,
> ref'ing it would be redundant. Instead, I just removed the superfluous
> CGContextRelease.
> 
> The attaches patch fixes the 7 crashes in the test suite, and the crashes in
> the application I'm working on.

The patch looks good (and it has already been pushed to master).
Comment 3 Michael Hutchinson 2013-03-29 18:06:24 UTC
>Actually I still cannot reproduce the issue (fun fact: I don't get the crash even >if I release the cgDrawContext multiple times, so there might be some kind of >auto-ignore in the libs I'm using).
>I'm running Lion (10.7.5) with an up-to-date development environment.
>What environment should I use to reproduce the crashes on >9e0748e223cfb8c5557c73f3ab5068ec1323e7c2 ?
>
>Side note: the CGContextRelease(status->cgDrawContext) is hit in 21 tests, yet >only 7 were crashing, so there might be something more to be fixed.

I'm running Mountain Lion 10.8.3 with Xcode 4.6.1, with an autotools toolchain and Cairo/GTK+/Mono stack built by our bockbuild build system.

To reproduce my build:
1) clone https://github.com/mono/bockbuild
2) checkout commit 276f40b6ab296bbc44785c8a55dcfb8fff786c64 (newer commits have my cairo patch applied)
3) build the profile: "cd profiles/monodevelop-mac-dev; make" (you can abort the build when it's reached cairo)
4) source the newly built environment: "source md-dev-env"
5) run cairo tests: "cd build-root/cairo-1.12.14; make test"
Comment 4 Andrea Canciani 2013-03-30 14:43:29 UTC
(In reply to comment #3)
> >Actually I still cannot reproduce the issue (fun fact: I don't get the crash even >if I release the cgDrawContext multiple times, so there might be some kind of >auto-ignore in the libs I'm using).
> >I'm running Lion (10.7.5) with an up-to-date development environment.
> >What environment should I use to reproduce the crashes on >9e0748e223cfb8c5557c73f3ab5068ec1323e7c2 ?
> >
> >Side note: the CGContextRelease(status->cgDrawContext) is hit in 21 tests, yet >only 7 were crashing, so there might be something more to be fixed.
> 
> I'm running Mountain Lion 10.8.3 with Xcode 4.6.1, with an autotools
> toolchain and Cairo/GTK+/Mono stack built by our bockbuild build system.
> 
> To reproduce my build:
> 1) clone https://github.com/mono/bockbuild
> 2) checkout commit 276f40b6ab296bbc44785c8a55dcfb8fff786c64 (newer commits
> have my cairo patch applied)
> 3) build the profile: "cd profiles/monodevelop-mac-dev; make" (you can abort
> the build when it's reached cairo)

I reach cairo, but it crashes with the message:
Traceback (most recent call last):
  File "./monodevelop-mac-dev.py", line 22, in <module>
    MonoDevelopMacDevProfile ().build ()
  File "../../bockbuild/profile.py", line 147, in build
    exec compile (open (path).read (), path, 'exec')
  File "./../../packages/cairo.py", line 35, in <module>
    CairoPackage ()
  File "./../../packages/cairo.py", line 3, in __init__
    CairoGraphicsPackage.__init__ (self, 'cairo', '1.12.14')
TypeError: unbound method __init__() must be called with CairoGraphicsPackage instance as first argument (got CairoPackage instance instead)
make: *** [all] Error 1

> 4) source the newly built environment: "source md-dev-env"

and I get no md-dev-env.

> 5) run cairo tests: "cd build-root/cairo-1.12.14; make test"

I tried to do the same thing on 4a17e295ecf40fae59f9fba0b59453223615c6a3 commenting out 'patches/cairo-quartz-crash.patch' and finally managed to reproduce the crashes!

It looks like it is sufficient to use different env settings to trigger the crashes.

I strongly suspect that it happens because of the 32-bits build; I will check when I have more time.
In any case, thanks a lot for the feedback!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.