Re: Release planning for 0.14.2

I will re-work on this.  There is a memory leak in caio_surface_flush () in xlib surface.  Observed that 5000 cairo_surface_flush () calls leaks about 2 MB.

Thanks for feedback

-----Original Message-----
From: Bryce Harrington [mailto:[email protected]] 
Sent: Saturday, March 07, 2015 1:56 PM
To: Uli Schlachter
Cc: Henry (Yu) Song; [email protected]
Subject: Re: [cairo] Release planning for 0.14.2

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 <[email protected]>
> > 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
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo