On Thu Jan 09 05:34:33 2014, MJEVANS wrote:
Show quoted text> On Fri Jan 03 18:57:04 2014, DOUGW wrote:
> > Here's a self-contained example.
> > The s/// seems to trigger the rebinding and more of an explosion of
> > size.
> >
> > my $sql = <<SQL;
> > DECLARE
> > num NUMBER;
> > BEGIN
> > num := :foo;
> > :foo := num + 1;
> > END;
> > SQL
> >
> > my $sth = $dbh->prepare($sql);
> >
> > $sth->bind_param_inout(':foo', \my $foo, 12);
> > $sth->trace(9);
> >
> > my $i;
> > for (0..1000) {
> > $i++;
> > $foo = $_;
> > $foo =~ s/.*// if $i % 2 == 0;
> > print "Calling w/$foo\n";
> > $sth->execute();
> > print "=> $foo\n";
> > }
>
> Thank you for this example.
>
> I have had a chance to have a quick look at this now and it appears
> memory is not being "leaked" as such but the svs used for output
> parameters as growing in size on each execute.
>
> I think this is down to a combination of some silly code and the code
> calling SvGROW which can grow your sv's buffer by more than you asked
> (since Perl 5.8.8). However, a fix is not so straight forward so bear
> with me.
>
> The only way you can avoid the output parameter growing right now is
> to rebind it to another scalar.
>
> You can find me on irc.perl.org in #dbi as mje if you want to talk
> about this.
>
> Martin
Try the attached patch applied against the git master Yanick looks after and pointed to in the DBD::Oracle distribution.
Martin
--
Martin J. Evans
Wetherby, UK
diff --git a/dbdimp.c b/dbdimp.c
index 63f0c45..e7c9556 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -2545,19 +2545,26 @@ dbd_rebind_ph_char(imp_sth_t *imp_sth, phs_t *phs)
if (imp_sth->ora_pad_empty)
croak("Can't use ora_pad_empty with bind_param_inout");
if (SvTYPE(phs->sv)!=SVt_RV || !at_exec) {
-
if (phs->ftype == 96){
- SvGROW(phs->sv,(STRLEN) (unsigned int)phs->maxlen-1);
+ SvGROW(phs->sv,(STRLEN) (unsigned int)phs->maxlen-1);
+ if (DBIc_DBISTATE(imp_sth)->debug >= 6 || dbd_verbose >= 6) {
+ PerlIO_printf(DBIc_LOGPIO(imp_sth),
+ "Growing 96 phs sv to %ld resulted in buffer %ld\n", phs->maxlen - 1, SvLEN(phs->sv));
+ }
} else {
STRLEN min_len = 28;
(void)SvUPGRADE(phs->sv, SVt_PVNV);
- /* ensure room for result, 28 is magic number (see sv_2pv) */
- /* don't apply 28 char min to CHAR types - probably shouldn't */
- /* apply it anywhere really, trying to be too helpful. */
- /* phs->sv _is_ the real live variable, it may 'mutate' later */
- /* pre-upgrade to high'ish type to reduce risk of SvPVX realloc/move */
- SvGROW(phs->sv, (STRLEN)(((unsigned int) phs->maxlen <= min_len) ? min_len : (unsigned int) phs->maxlen)+1/*for null*/);
-
+ /* ensure room for result, 28 is magic number (see sv_2pv) */
+ /* don't apply 28 char min to CHAR types - probably shouldn't */
+ /* apply it anywhere really, trying to be too helpful. */
+ /* phs->sv _is_ the real live variable, it may 'mutate' later */
+ /* pre-upgrade to high'ish type to reduce risk of SvPVX realloc/move */
+ /* NOTE SvGROW resets SvOOK_offset and we want to do this */
+ SvGROW(phs->sv, (STRLEN)(((unsigned int) phs->maxlen <= min_len) ? min_len : (unsigned int) phs->maxlen));
+ if (DBIc_DBISTATE(imp_sth)->debug >= 6 || dbd_verbose >= 6) {
+ PerlIO_printf(DBIc_LOGPIO(imp_sth),
+ "Growing phs sv to %ld resulted in buffer %ld\n", phs->maxlen +1, SvLEN(phs->sv));
+ }
}
}
@@ -2587,7 +2594,11 @@ dbd_rebind_ph_char(imp_sth_t *imp_sth, phs_t *phs)
phs->maxlen = 4000; /* Just make is a varchar max should be ok for most things*/
} else {
- phs->maxlen = ((IV)SvLEN(phs->sv)); /* avail buffer space (64bit safe) Logicaly maxlen should never change but it does why I know not*/
+ if (DBIc_DBISTATE(imp_sth)->debug >= 6|| dbd_verbose >= 6 ) {
+ PerlIO_printf(DBIc_LOGPIO(imp_sth),
+ "Changing maxlen to %ld\n", SvLEN(phs->sv));
+ }
+ phs->maxlen = ((IV)SvLEN(phs->sv)); /* avail buffer space (64bit safe) Logicaly maxlen should never change but it does why I know not - MJE because SvGROW can allocate more than you ask for - anyway - I fixed that and it doesn't grow anymore */
}
@@ -3261,11 +3272,16 @@ dbd_bind_ph(SV *sth, imp_sth_t *imp_sth, SV *ph_namesv, SV *newvalue, IV sql_typ
sv_setsv(phs->sv, newvalue);
if (SvAMAGIC(phs->sv)) /* overloaded. XXX hack, logic ought to be pushed deeper */
sv_pvn_force(phs->sv, &PL_na);
- } else if (newvalue != phs->sv) {
- if (phs->sv)
- SvREFCNT_dec(phs->sv);
+ } else {
+ if (newvalue != phs->sv) {
+ if (phs->sv)
+ SvREFCNT_dec(phs->sv);
- phs->sv = SvREFCNT_inc(newvalue); /* point to live var */
+ phs->sv = SvREFCNT_inc(newvalue); /* point to live var */
+ }
+ /* Add space for NUL - do it now rather than in rebind as it cause problems
+ in rebind where maxlen continually grows. */
+ phs->maxlen = phs->maxlen + 1;
}
return dbd_rebind_ph(sth, imp_sth, phs);
diff --git a/oci8.c b/oci8.c
index 6a3208b..46a69e8 100644
--- a/oci8.c
+++ b/oci8.c
@@ -1226,7 +1226,7 @@ dbd_phs_out(dvoid *octxp, OCIBind *bindp,
sv_setpv(sv,"");
}
- *bufpp = SvGROW(sv, (size_t)(((phs->maxlen < 28) ? 28 : phs->maxlen)+1)/*for null*/);
+ *bufpp = SvGROW(sv, (size_t)(((phs->maxlen < 28) ? 28 : phs->maxlen)));
phs->alen = SvLEN(sv); /* max buffer size now, actual data len later */
}
RT for rt.cpan.org