<html>
    <head>
      <base href="https://bugs.freedesktop.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Assertion "(_cairo_atomic_int_get (&(&surface->ref_count)->ref_count) > 0)""
   href="https://bugs.freedesktop.org/show_bug.cgi?id=91967#c26">Comment # 26</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Assertion "(_cairo_atomic_int_get (&(&surface->ref_count)->ref_count) > 0)""
   href="https://bugs.freedesktop.org/show_bug.cgi?id=91967">bug 91967</a>
              from <span class="vcard"><a class="email" href="mailto:alberts.muktupavels@gmail.com" title="Alberts Muktupāvels <alberts.muktupavels@gmail.com>"> <span class="fn">Alberts Muktupāvels</span></a>
</span></b>
        <pre>(In reply to Jaroslav Škarvada from <a href="show_bug.cgi?id=91967#c25">comment #25</a>)
<span class="quote">> (In reply to Alberts Muktupāvels from <a href="show_bug.cgi?id=91967#c24">comment #24</a>)
> > (In reply to Jaroslav Škarvada from <a href="show_bug.cgi?id=91967#c23">comment #23</a>)
> > > 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.</span >

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.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the QA Contact for the bug.</li>
      </ul>
    </body>
</html>