Re: memory leak in DBI XS bootstrap code
am 12.07.2006 17:42:10 von Tim.BunceOn Wed, Jul 12, 2006 at 05:30:47AM -0700, Ephraim Dan wrote:
> I am 99% sure that it's DBI that's leaking. We were using DBI 1.32 previously, and the same script does not leak when 1.32 is installed. Changing only DBI to 1.51 produces the leak. No DBDs were touched.
>
> I think it's isolated to connect and prepare:
>
> 1. When the loop consists only of connect and disconnect, 1 SV is leaked.
> 2. When the loop is connect, prepare and disconnect, 4 SVs are leaked.
> 3. When the connect is done only once, and the loop is only prepare, 1 SV is leaked.
> 4. When connect and prepare are outside the loop, and the loop consists of execute, fetchrow_array, finish, 0 SVs are leaked.
>
> I also tried it with DBD::Sponge, and the same numbers resulted.
>
> I have tried various attempts at finding this leak, but to no avail.
> I am pretty sure it's in the XS code, but I can't be sure. I tried
> using Devel::Cycle to see if it's a circular reference, but it didn't
> show anything. I don't know of other ways to make a memory leak in
> pure perl, which is why I suspect XS.
>
> I tried to use Devel::LeakTrace to find where the leaks are, but I'm
> not so sure of its results. For what it's worth, it seems to report
> leaks within DBI.pm in the following subroutines:
>
> connect() (return $dbh) install_driver() (eval qq{ DBI::_firesafe
> require $driver_class }, $DBI::installed_drh{$driver} = $drh)
> setup_driver() (push @{"${class}_mem::ISA"}, $mem_class) _new_handle()
> (DBI::_setup_handle($h, $imp_class, $parent, $imp_data))
>
> Do you have any other pointers on how to track this down? I am more
> than willing to try more things, but I've run out of ideas. I have
> tried valgrind but I don't think it helps with perl refcount issues,
> since they are not true memory leaks in the sense of lost pointers to
> malloc'ed buffers... In any event this has been a dead end so far...
>
> Thanks for any help you can give, Tim (or anyone else, please?)
You've reminded me that there's a memleak patch from Doru
Theodor Petrescu in the current development version. 1 sv per sth:
--- dbi/trunk/DBI.xs (original)
+++ dbi/trunk/DBI.xs Mon Jun 12 13:24:19 2006
@@ -1069,6 +1069,8 @@
SV *sv = av_shift(av);
if (SvOK(sv))
av_push(av, sv);
+ else
+ sv_free(sv); /* keep it leak-free by Doru Petrescu pdoru.dbi@from.ro */
}
}
}
Also, just above there is the line:
if (av_len(av) % 120 == 0) {
try changing that to
if (1) {
and recheck the leaks. If that seems to have fixed one then it's not
really a leak, just some caching that the DBI does.
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.
Tim.
> -edan
>
> > -----Original Message-----
> > From: Tim Bunce [mailto:Tim.Bunce@pobox.com]
> > 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:Tim.Bunce@pobox.com]
> > > > 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:Tim.Bunce@pobox.com]
> > > > > > 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:Tim.Bunce@pobox.com]
> > > > > > > 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:Tim.Bunce@pobox.com]
> > > > > > > > > > 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
> > > > > > distribution */
> > > > > > > > > > > #include
> > > > > > 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:Tim.Bunce@pobox.com
> > > > > > > 2. http://valgrind.org/
> > > > > > > 3. http://valgrind.org/