[cairo] Release planning for 0.14.2
Bryce Harrington
bryce at osg.samsung.com
Sat Mar 7 13:55:37 PST 2015
On Sat, Mar 07, 2015 at 10:36:42AM +0100, Uli Schlachter wrote:
> Hi,
>
> @Bryce: I would suggest reverting this
Hi Uli, thanks for the suggestion, I've reverted the change for the
release.
> Am 05.03.2015 um 01:20 schrieb Henry (Yu) Song:
> [...]
> > Following patch fixes a memory leak in xlib surface.
>
> this might be a stupid question, but: Which memory leak exactly? And why doesn't
> the commit message say so?
>
> All that _XEReadEvents() does is flushing the output buffer and reading in
> events, appending them to the internal queue of pending events (_XReadEvents ->
> poll_for_response() -> poll_for_event(), then back to _XReadEvents() ->
> handle_response() -> _XEnq()). So no memory leak here.
>
> Your code however calls _XDeq() on the first event on the queue. This function
> just removes the event from the list, but does not free it. So as I see it, this
> function is *introducing* a memory leak. And it even does so in plain sight,
> without having any other effect.
>
> (And also I have no idea why it would be a good idea to drop the whole event
> queue. Pretty much none of the events in there belong to us. Let's imagine an
> application mapping a window and then calling into cairo. Cairo will happily
> drop the MapNotify event and also any Expose events which means that the
> application will possibly not draw its window.)
>
> Could you please enlighten me?
>
> > From cdd185b6143876a4ce320e4f3a3f59d3fc7a5813 Mon Sep 17 00:00:00 2001
> > From: Henry Song <henrysong at samsung.com>
> > Date: Wed, 4 Mar 2015 10:06:36 -0800
> > Subject: [PATCH] xlib: Remove queued event from _XReadEvents
>
> This sounds like it should only remove the events that _XReadEvents() just
> enqueued. This would still be a stupid idea, but the patch drops the whole
> queue, no matter if part of it existed before _XReadEvents() was called.
>
> > ---
> > src/cairo-xlib-surface-shm.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/src/cairo-xlib-surface-shm.c b/src/cairo-xlib-surface-shm.c
> > index fa7d3eb..9b13505 100644
> > --- a/src/cairo-xlib-surface-shm.c
> > +++ b/src/cairo-xlib-surface-shm.c
> > @@ -670,6 +670,7 @@ _cairo_xlib_shm_surface_flush (void *abstract_surface, unsigned flags)
> > cairo_xlib_shm_surface_t *shm = abstract_surface;
> > cairo_xlib_display_t *display;
> > Display *dpy;
> > + _XQEvent *qev;
> > cairo_status_t status;
> >
> > if (shm->active == 0)
> > @@ -694,6 +695,10 @@ _cairo_xlib_shm_surface_flush (void *abstract_surface, unsigned flags)
> > while (! seqno_passed (shm->active, LastKnownRequestProcessed (dpy))) {
> > LockDisplay(dpy);
> > _XReadEvents(dpy);
> > + while (dpy->head) {
> > + qev = dpy->head;
> > + _XDeq (dpy, NULL, qev);
> > + }
> > UnlockDisplay(dpy);
> > }
> >
> >
>
> --
> "Every once in a while, declare peace. It confuses the hell out of your enemies"
> - 79th Rule of Acquisition
More information about the cairo
mailing list