Re: [Plplot-devel] Plplot init / end problem

[Alan W. I. <ir...@be...>]

On 2004-01-15 11:37-0000 Andrew Ross wrote:

> On Thu, Jan 15, 2004 at 12:20:05PM +0100, Rafael Laboissiere wrote:
> > * Andrew Ross <an...@co...> [2004-01-15 10:16]:
> >
> > > Unfortunately that doesn't fit in with the C++ philosophy which implicitly
> > > calls plend1() from the constuctor if this is not the last active stream
> > > and once all streams have been closed it calls plend(). However there is no
> > > guarantee that the user won't then want to open another stream. I think
> > > plend() should leave the system in a state where we can restart the library
> > > with plinit() and not have to exit the application and re-run it. If other
> > > disagree then maybe we need to change the C++ bindings so that it doesn't
> > > call plend() at all and the use has to do that explicitly. That will be
> > > horribly ugly though. It would have to be via the C API since the user may
> > > not have any C++ plstream objects left to call plend() with.
> >
> > These are good arguments.  Would your patch regarding the npldynamicdevices
> > and nloadabledrivers variables fix the problem *_without_* introducing any
> > undesirable side effects in the behavior of the PLplot library?  If yes,
> > then I would consider applying the patch to CVS.
>
> The patch should make absolutely no difference the first time you
> call pllib_init since the variables are initialised to zero in the
> header file plcore.h anyway. It is only on subsequent calls to
> pllib_init after calling plend that they will have any effect. So
> anyone who has a currently working program should notice no
> difference.

I repeat here Andrew's small patch so we can all see it without referring to
the attachment:

*********
diff -ur plplot-5.2.1.cvs.20040104/src/plcore.c plplot-5.2.1.cvs.20040104-fix/sr
c/plcore.c
--- plplot-5.2.1.cvs.20040104/src/plcore.c      Wed Dec 31 01:34:32 2003
+++ plplot-5.2.1.cvs.20040104-fix/src/plcore.c  Thu Jan 15 08:59:42 2004
@@ -1289,6 +1289,7 @@
 /* Release the libltdl resources */
     lt_dlexit();
 #endif
+    lib_initialized = 0;
 }

 /*--------------------------------------------------------------------------*\
@@ -1652,6 +1653,10 @@
     DIR* dp_drvdir = NULL;
     struct dirent* entry;
     /* lt_dlhandle dlhand; */
+
+    /* Make sure driver counts are zeroed */
+    npldynamicdevices = 0;
+    nloadabledrivers = 0;

 /* Open a temporary file in which all the plD_DEVICE_INFO_<driver> strings
    will be stored */
*********

There are two aspects to this patch.

** There is the plend change rezeroing lib_initialized which allows a user
to reinitialize the library.  It makes sense to me to allow the user to
start fresh again like this.  However, Andrew, you should be aware that
there are memory management issues still in the dynamic drivers code.
(Memory keeps being used up by malloc and never freed, and in one case freed
memory is accessed, if I recall correctly the detailed leak-check analysis
that is available from valgrind.) So far we have got away with not dealing
with these problems, but they may come back to haunt us when you allow the
user to re-initialize the library many times.  The unfreed memory will just
keep increasing each time you do this ==> memory leak, and at some point the
access to memory which has already been freed will kill us.  So that is a
downside, but I am willing to accept this additional risk for now since I
don't understand the dynamic drivers code well enough to deal with the
memory management issues there.

** Initializing npldynamicdevices and nloadabledrivers.

Andrew, can you assure us you understand that dynamic drivers code well
enough to be sure this is the right thing to do?  (I believe this was the
question Rafael was asking.) If so, then I think we should try your patch
as is to see if there is no obvious breakage in all our standard tests.

But that brings up an additional (post-release?) issue:

If you do understand that dynamic drivers code well enough to keep track of
initialization, then I hope you would be willing to also look at the memory
management issues as well.  In particular, the goal should be after plend is
executed, there is absolutely no malloc memory left to be freed. I highly
recommend using valgrind on the installed examples to help diagnose exactly
where the memory management problems occur.  (Note, valgrind does not work
on examples built in the build tree with "make check" since those are
implemented with scripts.)

For example, if you run valgrind on x01c:

valgrind ./x01c -dev ps -o temp.ps
==29647== Memcheck, a.k.a. Valgrind, a memory error detector for x86-linux.
==29647== Copyright (C) 2002-2003, and GNU GPL'd, by Julian Seward.
==29647== Using valgrind-20030725, a program supervision framework for
x86-linux.
==29647== Copyright (C) 2000-2003, and GNU GPL'd, by Julian Seward.
==29647== Estimated CPU clock rate is 604 MHz
==29647== For more details, rerun with: -v
==29647==
PLplot library version: 5.2.1.cvs.20040104
==29647== discard syms in
/home/software/plplot_cvs/install_location_alternative/lib/plplot5.2.1.cvs.20040104/driversd/ps.so
due to munmap()
==29647==
==29647== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==29647== malloc/free: in use at exit: 4647 bytes in 239 blocks.
==29647== malloc/free: 309 allocs, 70 frees, 82275 bytes allocated.
==29647== For a detailed leak analysis,  rerun with: --leak-check=yes
==29647== For counts of detected errors, rerun with: -v


Note the disparity between the number of allocs and frees, and the 4647
bytes still left in the 239 different mallocs that have not been freed at
plend time.  If you run with --leak-check=yes, --gdb-attach=yes, and
--num-callers=40, there are a lot more detailed checks that occur to help
pin-point the source of the problem.

Alan
__________________________
Alan W. Irwin
email: ir...@be...
phone: 250-727-2902

Astronomical research affiliation with Department of Physics and Astronomy,
University of Victoria (astrowww.phys.uvic.ca).

Programming affiliations with the PLplot scientific plotting software
package (plplot.org), the Yorick front-end to PLplot (yplot.sf.net), the
Loads of Linux Links project (loll.sf.net), and the Linux Brochure Project
(lbproject.sf.net).
__________________________

Linux-powered Science
__________________________