Created attachment 78313 [details]
Stack trace of bug
I get an assertion failure in cairo-surface.c, line 390 "CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&surface->ref_count)", when using the Gimp's new (in development) canvas rotation feature on windows.
The issue seems to be in the way the windows backend handles references when an image is destroyed.
The relevant code in the gimp is here:
I have attached a stacktrace.
Someone should paint a picture about this, the reference magic makes my head dizzy:
At the cairo_restore() at the end of gimp_display_shell_render(), the current source of the gc is a surface pattern. In the rotated case, this pattern holds the only reference to the surface "xfer" which is a similar image to cairo_get_target(cr) (a win32 surface). Behind the covers, this image surface has a reference to a win32_display_surface (its parent).
Now when the pattern is destroyed, the reference count to the similar image drops to zero (This seems correct, although the behind-the-scenes reference loop with the win32_display_surface still confuses me). While finishing this image surface, it drops the reference to its parent. Thus, now the win32_display_surface gets finished, too. In _cairo_win32_display_surface_finish(), this one now tries to get rid of the similar image again, although this one is already being finished some stack frames above. And *boom*, assert() notices that something went foobar.
Whoever maintains cairo-win32 can come up with a good solution. Since these reference loops make my head hurt, I can only suggest to make _cairo_image_surface_finish() set ->parent to NULL before destroying its parent and then in _cairo_win32_display_surface_finish() check if the image surface's parent is still set before destroying the image. However, this hack smells quite fishy to me.
Created attachment 82797 [details] [review]
[PATCH] win32: Fix image surface freeing loop
Well, this is another way to fix the bug. This patch simply checks to see if the reference count is zero on the parent surface, and if it is, it won't try to free it.
I've tested the patch, and can confirm that it works.
I think I can also confirm that Michael's patch also fixes the use of GTK+ master on Windows .
With blessings, thank you!
I'm not keen on using the ref-count here, and so used a more explicit check instead. Hopefully, this should keep the reference checking logic intact.
Author: Chris Wilson <email@example.com>
Date: Fri Aug 23 12:48:08 2013 +0100
win32: Prevent double-free of similar images
Based on a patch and analysis by Michael Henning.
When we create a similar-image surface for win32, we set up a couple of
back references from the image to the win32 surface, and vice versa. We
need to be careful when decoupling the reference cycle to avoid chasing
around the loop upon destruction. Currently we handled destroying the
similar-image via the parent win32 surface, but similar precaution is
required when destroying the surface via the similar-image.
Reported-by: Michael Henning <firstname.lastname@example.org>
Signed-off-by: Chris Wilson <email@example.com>
I can confirm that the fix works. Thanks!
Thank you!!! This has finally fixed #49755, which has made Cairo unusable on Windows all through 1.12.x.