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: https://git.gnome.org/browse/gimp/tree/app/display/gimpdisplayshell-render.c?id=0909a30b06f4cf856792a27fecc606e674b72716#n48 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.
Hi, I think I can also confirm that Michael's patch also fixes the use of GTK+ master on Windows [1]. [1]: https://bugzilla.gnome.org/show_bug.cgi?id=704540 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. commit fb8881e84bb24b2a54ee5aa449b6f5638de36404 Author: Chris Wilson <chris@chris-wilson.co.uk> 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 <drawoc@darkrefraction.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63787 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
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.
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.