[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 #6 from Bryce Harrington <b.harrington at samsung.com> ---
A couple things...

First, if we set image to NULL and it hits BAIL, then doesn't it just crash
unreferencing it here?

    if (unlikely (status)) {
        cairo_surface_destroy (&image->base);
        return _cairo_surface_create_in_error (status);
    }


If image can be NULL, then there are a few places in the function where
&image->base is used that would need NULL ptr checks else they might segfault.

As well, I would think the image object would need to be deliberately
destroyed, to avoid memory leak.  I.e.:

...
        cairo_surface_destroy (&image->base);
        cairo_surface_destroy (image);
        image = NULL;
    }
...

However, backing up a bit... if XShmGetImage() has failed, such that we're
destroying &image->base, is any of the rest of the function still valid?  I
notice from your trace that it's just hitting another error and bouncing down
to BAIL, where the crash occurs.  It looks like most of the BAIL logic doesn't
apply here since the objects its freeing hadn't been created.  Which makes me
wonder if when this error situation is hit, if it'd be better to just exit the
function?  Then we can avoid worrying about NULL image pointers and what's
happening in BAIL.

...
        cairo_surface_destroy (&image->base);
        cairo_surface_destroy (image);
        return _cairo_surface_create_in_error (CAIRO_STATUS_INVALID_VISUAL);
    }
...

Maybe the error code there should be CAIRO_STATUS_NO_MEMORY instead.

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