> You could also try another change a few lines further up where it says
>
> #ifdef sv_rvweaken
> if (1) {
>
> Change the 1 to a 0 to disable the use of weakrefs which was added in
> 1.49.
[edan] This seems to have done it! It fixes all the leaks! Hurrah! So... do
I need to keep this patched myself, at the risk of breaking when the code
changes in some future release, or can you somehow make this optional, or fix
it, or explain to me what I can do to make the leak go away using the current
code?
Thanks so much,
edan
> > > -----Original Message-----
> > > From: Tim Bunce [mailto:[EMAIL PROTECTED]
> > > Sent: Tuesday, July 11, 2006 16:59
> > > To: Ephraim Dan
> > > Cc: Tim Bunce; dbi-users@perl.org
> > > Subject: Re: memory leak in DBI XS bootstrap code
> > >
> > > I'm sorry but I simply don't have the time at the moment.
> > >
> > > Some random suggestions:
> > > - try using different sets of methods to isolate the minimum
> > > requirements.
> > > for example:
> > > call fetchrow_array just once.
> > > don't call fetchrow_array at all.
> > > call fetchrow_array in loop but don't call finish
> > > don't execute, fetch, finish.
> > > etc etc etc
> > > - try with other drivers to see if it's the driver or the DBI that's
> > > leaking
> > >
> > > Tim.
> > >
> > > On Tue, Jul 11, 2006 at 03:59:05AM -0700, Ephraim Dan wrote:
> > > > Tim,
> > > >
> > > > I'm experiencing further memory leaks which are causing me more
> grief.
> > > I put together the following test script which illustrates the
> leakage:
> > > >
> > > > #/usr/bin/perl
> > > > # dbi.pl
> > > >
> > > > use DBI;
> > > > use Devel::Leak;
> > > > my $leak_handle;
> > > >
> > > > for ( 1 .. 1000 ) {
> > > > my $dbh = DBI->connect("DBI:mysql:MYDB","user","secret",
> > > {RaiseError=>1});
> > > > my $sth = $dbh->prepare("SELECT COUNT(*) FROM TABLE");
> > > > $sth->execute();
> > > > while ( my ($c) = $sth->fetchrow_array() ) {
> > > > # print "$c\n";
> > > > }
> > > > $sth->finish;
> > > > $dbh->disconnect;
> > > >
> > > > system("ps -aux | fgrep dbi.pl | fgrep -v fgrep");
> > > > test_leak();
> > > > }
> > > >
> > > > sub test_leak {
> > > > if ( $handle ) {
> > > > Devel::Leak::CheckSV($handle);
> > > > }
> > > > print STDERR "count: " . Devel::Leak::NoteSV($handle) . "\n";
> > > > }
> > > >
> > > > Devel::Leak shows that it is leaking 4 "things" every iteration:
> > > >
> > > > count: 12128
> > > > root 17720 17.8 0.3 6840 4220 pts/0 S 09:55 0:00 perl
> > > dbi.pl
> > > > new 0x8aad3e8 : SV = PVHV(0x8a68b30) at 0x8aad3e8
> > > > REFCNT = 1
> > > > FLAGS = (PADBUSY,PADMY,SHAREKEYS)
> > > > IV = 0
> > > > NV = 0
> > > > ARRAY = 0x0
> > > > KEYS = 0
> > > > FILL = 0
> > > > MAX = 7
> > > > RITER = -1
> > > > EITER = 0x0
> > > > new 0x8aad3f4 : SV = NULL(0x0) at 0x8aad3f4
> > > > REFCNT = 1
> > > > FLAGS = (PADBUSY,PADMY)
> > > > new 0x8aad400 : SV = NULL(0x0) at 0x8aad400
> > > > REFCNT = 1
> > > > FLAGS = (PADBUSY,PADMY)
> > > > new 0x8aad40c : SV = NULL(0x0) at 0x8aad40c
> > > > REFCNT = 1
> > > > FLAGS = (PADBUSY,PADMY)
> > > > count: 12132
> > > >
> > > > I think there might be several leaks here. If I move the connect()
> out
> > > of the loop, it's leaking, but less - just one of those NULL SV guys.
> > > >
> > > > Can you please help me track this down? I'm still a little scared
> of
> > > the DBI.xs internals, despite my recent exposure...
> > > >
> > > > Thanks a lot,
> > > > edan
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Tim Bunce [mailto:[EMAIL PROTECTED]
> > > > > Sent: Monday, July 03, 2006 12:16
> > > > > To: Ephraim Dan
> > > > > Cc: Tim Bunce; dbi-users@perl.org
> > > > > Subject: Re: memory leak in DBI XS bootstrap code
> > > > >
> > > > > On Sat, Jul 01, 2006 at 10:42:58PM -0700, Ephraim Dan wrote:
> > > > > > 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.
> > > > >
> > > > > Looks okay at first sight, except that Newz returns memory that's
> been
> > > > > reset to zero bytes. You need to add the memzero I suggested
> > > previously.
> > > > > Might as well go into malloc_using_sv since it's not performance
> > > critical.
> > > > >
> > > > > Um, looks like CLONE plus _clone_dbis plus dbi_bootinit already do
> the
> > > > > right thing (your patch also fixes a leak on thread creation) so
> > > that's
> > > > > okay. INIT_PERINTERP is harder to be sure about, but I think it's
> > > okay.
> > > > >
> > > > > I'll apply the patch and add the memzero. Thanks edan.
> > > > >
> > > > > Tim.
> > > > >
> > > > > > 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/
