[cairo] Release planning for 0.14.2

Uli Schlachter psychon at znc.in
Sat Mar 7 01:36:42 PST 2015


Hi,

@Bryce: I would suggest reverting this

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