Thanks for the help Tim! You've really done me a great service.
I have created the following patch, which appears to fix all of the leaks. Can
you please review it, and give it your stamp of approval? I am willing to use
this patch if you approve it, until a new DBI is available with the fixes.
Thanks again,
edan
The patch:
--- DBI.xs.orig 2006-06-30 10:20:10.000000000 -0400
+++ DBI.xs 2006-07-02 04:59:32.000000000 -0400
@@ -171,7 +171,7 @@
dPERINTERP_SV; dPERINTERP_PTR(PERINTERP_t *, PERINTERP)
# define INIT_PERINTERP \
dPERINTERP; \
- Newz(0,PERINTERP,1,PERINTERP_t); \
+ PERINTERP = malloc_using_sv(sizeof(PERINTERP_t)); \
sv_setiv(perinterp_sv, PTR2IV(PERINTERP))
# undef DBIS
@@ -209,13 +209,29 @@
stc_s, sizeof(dbih_stc_t), fdc_s, sizeof(dbih_fdc_t), msg);
}
+static void *
+malloc_using_sv(STRLEN len)
+{
+ dTHX;
+ SV *sv = newSV(len);
+ return SvPVX(sv);
+}
+
+static char *
+savepv_using_sv(char *str)
+{
+ char *buf = malloc_using_sv(strlen(str));
+ strcpy(buf, str);
+ return buf;
+}
+
static void
dbi_bootinit(dbistate_t * parent_dbis)
{
dTHX;
INIT_PERINTERP;
- Newz(dummy, DBIS, 1, dbistate_t);
+ DBIS = (struct dbistate_st*)malloc_using_sv(sizeof(struct dbistate_st));
/* store version and size so we can spot DBI/DBD version mismatch */
DBIS->check_version = check_version;
@@ -3793,7 +3809,7 @@
ima->minargs = (U8)SvIV(*av_fetch(av, 0, 1));
ima->maxargs = (U8)SvIV(*av_fetch(av, 1, 1));
svp = av_fetch(av, 2, 0);
- ima->usage_msg = savepv( (svp) ? SvPV(*svp,lna) : "");
+ ima->usage_msg = (svp) ? savepv_using_sv(SvPV(*svp, lna)) : "";
ima->flags |= IMA_HAS_USAGE;
if (trace_msg && DBIS_TRACE_LEVEL >= 11)
sv_catpvf(trace_msg, ",\n usage: min %d, max %d, '%s'",
> -----Original Message-----
> From: Tim Bunce [mailto:[EMAIL PROTECTED]
> Sent: Friday, June 30, 2006 16:33
> To: Ephraim Dan
> Cc: Tim Bunce; dbi-users@perl.org
> Subject: Re: memory leak in DBI XS bootstrap code
>
> On Fri, Jun 30, 2006 at 04:08:46AM -0700, Ephraim Dan wrote:
> > This is why I love the perl community - you're willing to help even
> though you don't have time, since
> > you care about your code, and that other people can benefit from it.
> >
> > But ... you sort of left a dangling "But" in your last post. Do you
> have a suggestion to fix the
> > _install_method problem?
> >
> > I just ran valgrind with your fix - there seems to still be a leak.
> It is reported on DBI.xs line 216:
> > INIT_PERINTERP;
> > That looks to also have a Newz in it - is that what leaks?
>
> Yes. It's basically a plain malloc and there's no mechanism to free it
> later.
>
> > Can you suggest how to fix that one?
>
> The trick is to use newSV to alloc memory in the form of an SV
> so that when the interpreter is destroyed the SV memory gets freed
> automatically.
>
> > The _install_method leak is reported on line 3800 (in my modified
> DBI.xs) which validates your theory:
> > ima->usage_msg = savepv( (svp) ? SvPV(*svp,lna) : "");
> > If you can fix these, I'll be forever in your debt...
>
> As above, and as outlined previously, use an SV.
> Something like this (completely untested):
>
> static void *
> malloc_using_sv(STRLEN len) {
> SV *sv = newSV(len);
> return SvPVX(sv);
> }
>
> static char *
> savepv_using_sv(char *str) {
> char *buf = malloc_using_sv(strlen(str));
> strcpy(buf, str);
> return buf;
> }
>
> Tim.
>
> > --edan
> >
> > ---------------------------------------------------------------------
> -----------------------------------
> >
> > From: Tim Bunce [mailto:[EMAIL PROTECTED]
> > Sent: Fri 6/30/2006 11:35
> > To: Ephraim Dan
> > Cc: Tim Bunce; dbi-users@perl.org
> > Subject: Re: memory leak in DBI XS bootstrap code
> >
> > On Fri, Jun 30, 2006 at 12:24:33AM -0700, Ephraim Dan wrote:
> > > > > I don't see what you mean in the "INSTALL" that comes with perl
> 5.8.0 (that's what we're using).
> > > > The file called INSTALL in the perl source code directory.
> > >
> > > That I knew. What are the special instructions that I'm supposed
> to
> > > find there? Am I supposed to build it with debugging? Can you
> > > specify what special configuration exactly you meant that I should
> use?
> >
> > I'm sorry. I should have checked myself first. You can build perl
> with
> > -DPURIFY to enable more precise leak detection.
> >
> > > It looks like the leak in boot_DBI is on purpose:
> > > /* Remember the last handle used. BEWARE! Sneaky stuff here!
> */
> > > /* We want a handle reference but we don't want to increment
> */
> > > /* the handle's reference count and we don't want perl to try
> */
> > > /* to destroy it during global destruction. Take care!
> */
> > > DBI_UNSET_LAST_HANDLE; /* ensure setup the correct way
> */
> > >
> > > Why is this being done, and does anyone have a way to fix it? Why
> don't we want perl to destroy it?
> > Me, that's exactly what I want.
> >
> > I don't believe that's a leak. It's just using an SV as a reference
> > without telling perl its a reference. If perl knew it was a reference
> > we'd get double-free warnings as the referenced value would be freed
> twice.
> >
> > The leak is probably the DBIS structure itself:
> >
> > Newz(dummy, DBIS, 1, dbistate_t);
> >
> > That should probably be using along the lines of:
> >
> > sv = newSV(sizeof(struct dbistate_st));
> > DBIS = (struct dbistate_st*)SvPVX(sv);
> > memzero(DBIS, sizeof(struct dbistate_st));
> >
> > So the memory is allocated within an SV so gets freed on global
> destruction.
> >
> > > Still looking into the other leak in _install_method... any
> pointers still appreciated... Thanks for
> > your help so far, Tim.
> >
> > I've taken a look at the code. I'd guess that it's due to savepnv:
> >
> > ima->usage_msg = savepv( (svp) ? SvPV(*svp,lna) : "");
> >
> > But
> >
> > Tim.
> >
> > > -edan
> > >
> > > > > -----Original Message-----
> > > > > From: Tim Bunce [[1]mailto:[EMAIL PROTECTED]
> > > > > Sent: Thursday, June 29, 2006 14:47
> > > > > To: Ephraim Dan
> > > > > Cc: dbi-users@perl.org
> > > > > Subject: Re: memory leak in DBI XS bootstrap code
> > > > >
> > > > > Try building perl with options to make valgrind leak tracing
> more
> > > > > effective (see perl's INSTALL file). That may help you pinpoint
> > > > > the problem.
> > > > >
> > > > > Tim.
> > > > >
> > > > > On Thu, Jun 29, 2006 at 04:33:40AM -0700, Ephraim Dan wrote:
> > > > > > I am experiencing what I believe to be a memory leak in the
> DBI
> > > > > bootstrap code. This is a problem for me because I am
> embedding perl in a
> > > > > long-running program, and DBI is being loaded over and over, so
> my program
> > > > > grows and grows.
> > > > > >
> > > > > > The problem appears to be in the following routines:
> > > > > >
> > > > > > boot_DBI (in /usr/lib/perl5/vendor_perl/5.8.0/i386-linux-
> thread-
> > > > > multi/auto/DBI/DBI.so)
> > > > > > XS_DBI__install_method (in
> /usr/lib/perl5/vendor_perl/5.8.0/i386-linux-
> > > > > thread-multi/auto/DBI/DBI.so)
> > > > > >
> > > > > > I am using DBI 1.51
> > > > > >
> > > > > > The tool "valgrind" ([2]http://valgrind.org
> <[3]http://valgrind.org/> ) can be used to reproduce
> > the
> > > > > leak using the following code:
> > > > > >
> > > > > > ---
> > > > > > File: embed_test.c
> > > > > > ---
> > > > > >
> > > > > > #include <EXTERN.h> /* from the Perl
> distribution */
> > > > > > #include <perl.h> /* from the Perl
> distribution */
> > > > > >
> > > > > > static PerlInterpreter *my_perl; /*** The Perl
> interpreter ***/
> > > > > > EXTERN_C void xs_init (pTHX); /*** init dyn. loading
> ***/
> > > > > >
> > > > > > int main(int argc, char **argv, char **env)
> > > > > > {
> > > > > > char *embedding[] = { "", "-e", "0" };
> > > > > > my_perl = perl_alloc();
> > > > > > perl_construct(my_perl);
> > > > > > perl_parse(my_perl, xs_init, 3, embedding, (char
> **)NULL);
> > > > > > PL_exit_flags |= PERL_EXIT_DESTRUCT_END;
> > > > > > perl_run(my_perl);
> > > > > > eval_pv("use DBI", TRUE);
> > > > > > perl_destruct(my_perl);
> > > > > > perl_free(my_perl);
> > > > > > }
> > > > > >
> > > > > > ---
> > > > > > File: Makefile
> > > > > > ---
> > > > > >
> > > > > > CC_OPTS = $(shell perl -MExtUtils::Embed -e ccopts)
> > > > > > LD_OPTS = $(shell perl -MExtUtils::Embed -e ldopts)
> > > > > >
> > > > > > EXE = embed_test
> > > > > >
> > > > > > $(EXE): xsinit.o embed_test.o
> > > > > > gcc -o $(EXE) embed_test.o xsinit.o $(LD_OPTS)
> > > > > >
> > > > > > embed_test.o: embed_test.c
> > > > > > gcc -c embed_test.c $(CC_OPTS)
> > > > > >
> > > > > > xsinit.o: xsinit.c
> > > > > > gcc -c xsinit.c $(CC_OPTS)
> > > > > >
> > > > > > xsinit.c:
> > > > > > perl -MExtUtils::Embed -e xsinit -- -o xsinit.c
> > > > > >
> > > > > > clean:
> > > > > > rm -f *.o xsinit.c $(EXE)
> > > > > >
> > > > > > ---
> > > > > > EOF
> > > > > > ---
> > > > > >
> > > > > > Can anyone suggest a fix for this? I'd be more than willing
> to take a
> > > > > patch to DBI 1.51 as soon as someone has one.
> > > > > >
> > > > > > Thanks,
> > > > > > Ephraim Dan
> > > > > >
> > >
> >
> > References
> >
> > Visible links
> > 1. mailto:[EMAIL PROTECTED]
> > 2. http://valgrind.org/
> > 3. http://valgrind.org/
