Re: memory leak in DBI XS bootstrap code

On 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@xxxxxxx */
}
}
}

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@xxxxxxxxx]
Sent: Tuesday, July 11, 2006 16:59
To: Ephraim Dan
Cc: Tim Bunce; dbi-users@xxxxxxxx
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@xxxxxxxxx]
Sent: Monday, July 03, 2006 12:16
To: Ephraim Dan
Cc: Tim Bunce; dbi-users@xxxxxxxx
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@xxxxxxxxx]
Sent: Friday, June 30, 2006 16:33
To: Ephraim Dan
Cc: Tim Bunce; dbi-users@xxxxxxxx
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@xxxxxxxxx]
Sent: Fri 6/30/2006 11:35
To: Ephraim Dan
Cc: Tim Bunce; dbi-users@xxxxxxxx
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@xxxxxxxxx]
> > > Sent: Thursday, June 29, 2006 14:47
> > > To: Ephraim Dan
> > > Cc: dbi-users@xxxxxxxx
> > > 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:Tim.Bunce@xxxxxxxxx
2. http://valgrind.org/
3. http://valgrind.org/
.

Relevant Pages