> Did you try the other suggestions? > How much improvement did each give? (when testing by destroying the > interpreter)
Yeah, sorry I didn't give more detail on that. Here goes: The first change (Doru Petrescu): No change, still leaks 4 per connect/prepare/execute/fetch/disconnect loop. The (av_len(av) % 120 == 0) change: That does reduce it to 3 per loop, but I am not sure what you mean that it is not a leak - when it leaks 4, it's 4 every time - it's not stabilizing after some time, or jumping back down once in a while, or only leaking 3 sometimes, just going up and up by 4 every time. So how is that not a leak? As I said, the "if (0)" on the whole chunk of code produces squeaky clean iterations. -edan > > 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, > > I'd probably accept a patch that adds an option to Makefile.PL that then > passes a -DDBI_NO_WEAKREFS to the compiler which, with a suitable > #ifndef in DBI.xs, would disable that code. Or a new per-handle > flag could be added. > > But I'm reluctant to do either till I know more about the cause. > > > or explain to me what I can do to make the > > leak go away using the current code? > > I need more info - see above. > > Tim. > > > 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/
