On Wed, Mar 18, 2015 at 11:10:20PM +0000, Andrew Ross wrote:
> On Wed, Mar 18, 2015 at 10:57:33PM +0000, Andrew Ross wrote:
> > On Wed, Mar 18, 2015 at 12:59:18AM -0700, Alan Irwin wrote:
> > > @Andrew: There are some really interesting release implications here,
> > > and if you feel Norman's fix (ideally in git am form as I have
> > > requested from him below) or some other variant of it you like is a
> > > no-brainer please go ahead and push it (see my further comment on the
> > > release delay implications at the end.).
> >
> > Norman,
> >
> > I'm not sure I agree with your patch here in general. I've tested it on
> > Linux and it makes no difference to my valgrind results. Further, adding
> > in some debugging statements shows that at the point you set plsc->tidy
> > the dispatch table entry is NULL anyway so your code makes no difference.
> > This is with dynamic drivers. Are you using static drivers? In that case
> > things may be different and we need to investigate more closely.
> >
> > Also, looking at the code in plP_tidy, the tidy function
> > dispatch_table[dev - 1]->pl_tidy should already be called. Again debugging
> > statements show this is the case, so I can well see why you end up with a
> > double call for the psc driver if your change was having an effect.
> >
> > Can you confirm the build options you are using for plplot, and also the
> > precise way you ran valgrind to generate this feedback. Since I'm unable
> > to reproduce your results I'm a bit confused at precisely what the memory
> > issue is.
> >
>
> OK. To follow up my own post I think I now know what is going wrong. You
> fix is definitely wrong, because if you do compile without dynamic drivers
> then your code results in a double call to the tidy function. I was
> exiting the examples using return or 'Q', which results in stream_closed
> being set to 0 so the tidy function is called. If you exit by pressing the
> close button at the top of the xwin window however, then stream_closed = 1
> and so the tidy function is not called. This results in an additional memory
> leak. I'll have a look how to fix that.
>
> Norman, can you confirm that my interpretation of what you are doing is
> correct?
The attached patch fixes the memory leak for me. Pressing the close button
should now behave the same as pressing 'Q', i.e. it will abort the
program and clean up correctly. It looks like this was changed at some
stage in the past, but I don't know why. I guess this would allow a
programme to continue, and to potentially reopen a new xwin window. If we
want to retain this functionality it will require a bit more thinking
about.
The relevant commit is
commit 9961465cb7b90c07c216f1a708b67d7a65753e11
Author: Hazen Babcock <hbabc...@users.sourceforge.net>
Date: Mon May 31 00:46:56 2010 +0000
Change xwin driver to not call plexit() when the window is closed by
clicking on the close box. Instead all subsequent plotting commands sent
to the stream are ignored.
svn path=/trunk/; revision=11041
I'm a little uneasy about applying this patch so late in the release cycle
in case there are any unforseen consequences. The current situation will
result in a memory leak when closing the window with the button, but since
this is on exit anyway it should not be too serious.
Andrew
>From 131ba88d300d311600dbd187fa4e04b9ebd5f57d Mon Sep 17 00:00:00 2001
From: Andrew Ross <andrewr...@users.sourceforge.net>
Date: Wed, 18 Mar 2015 23:24:38 +0000
Subject: [PATCH] Fix up closing of xwin driver when pressing the close window
button to ensure the tidy function is called.
---
drivers/xwin.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xwin.c b/drivers/xwin.c
index 982d28f..9b2e018 100644
--- a/drivers/xwin.c
+++ b/drivers/xwin.c
@@ -1523,9 +1523,9 @@ ClientEH( PLStream *pls, XEvent *event )
if ( (Atom) event->xclient.data.l[0] == XInternAtom( xwd->display, "WM_DELETE_WINDOW", False ) )
{
pls->nopause = TRUE;
- pls->stream_closed = TRUE;
+ //pls->stream_closed = TRUE;
dev->exit_eventloop = TRUE;
- // plexit( "" );
+ plexit( "" );
}
}
--
2.1.0
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel
