[cairo-bugs] [Bug 91967] Assertion "(_cairo_atomic_int_get (&(&surface->ref_count)->ref_count) > 0)"

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=91967

--- Comment #27 from Jaroslav Škarvada <jskarvad at redhat.com> ---
(In reply to Alberts Muktupāvels from comment #26)
> (In reply to Jaroslav Škarvada from comment #25)
> > (In reply to Alberts Muktupāvels from comment #24)
> > > (In reply to Jaroslav Škarvada from comment #23)
> > > > But I think the proposed fix is dirty. It relies on the safety check inside
> > > > the cairo_surface_destroy. Cleanly written code shouldn't do this. The
> > > > control flow should never get into the cairo_surface_destroy for the second
> > > > time, that's why I wrote "maybe there is a better fix". But this is
> > > > definitely question for the upstream maintainers.
> > > 
> > > I don't want to agree on this.
> > > 
> > > Check what could happen if it is called this way:
> > > _get_image_surface (..., ..., FALSE);
> > > 
> > > I am too lazy to count, but there are definitely multiple paths that could
> > > end up calling cairo_surface_destroy (&image->base); when image is still
> > > NULL.
> > > 
> > > Think about this way - setting it to NULL after destroying is same as if
> > > that function would have been called with try_shm = FALSE.
> > 
> > See it this way:
> > 
> > - what about setting the surface NULL in the cairo_surface_destroy after
> > destroying it? It will fix all these issues, but it is apparently not the
> > right way how to fix such bugs.
> > 
> > I think that all the wrong paths leading to the second call of the
> > cairo_surface_destroy should be fixed/cleaned. But I am not upstream, so
> > it's irrelevant what I am thinking about it.
> 
> When try_shm == FALSE then cairo_surface_destroy still can be called when
> image == NULL and it will be first call...
> 
> Safety check in cairo_surface_destroy most likely is to save code / lines.
> It is easier to write simply cairo_surface_destory (surface); then if
> (surface) cairo_surface_destroy (surface);
> 
> Since cairo_surface_destroy is created NULL safe the problem is not that it
> is called second time. Problem is that it is called with already destroyed
> surface.

I am afraid you don't understand what I tried to express. And I am also afraid
that just repeating arguments will not move this bugzilla further.

For upstream maintainers: there are backtraces, reproducer and clear insight
what's going there. Could you take a look?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.cairographics.org/archives/cairo-bugs/attachments/20160524/1a0ad86e/attachment.html>