No DESTROY on untie. Tie memory leak fixed.

Subject: Re: Bug w/ DB_File: no flush on untie

On Sat, 17 May 1997 16:06:26 BST, Paul Marquess wrote:
>
>Good bug report Jay. Thanks.

I'll say!

>Turns out this isn't a DB_File problem but a tied hash/array problem.
>Running this script:
>
>    package fred ;
>    sub TIEHASH { return bless [] }
>    sub TIEARRAY { return bless [] }
>    sub FETCH { print "FETCH\n"}
>    sub STORE { print "STORE\n"}
>    sub CLEAR { print "CLEAR\n"}
>    sub DESTROY { print "DESTROY\n" }
>
>    package main ;
>
>    tie %x, 'fred' ;
>    %x = (1,2,3,4) ;
>    $x{2} = 3 ;
>    untie %x ;
>    print "untied\n" ;
>
>I got
>
>    CLEAR
>    STORE
>    STORE
>    STORE
>    untied
>    DESTROY
>
>The tied object isn't getting destroyed until global destruction at the
>end. That is a bug. I'd guess that something in the logic to do array
>assignments is holding on to the tied object and not letting go.
>
>The story is the same for tied arrays.
>
>Who said they were going to take on the tied stuff post 5.004?

That's partly on my plate.

This bug (a memory leak, actually) has been around since the beginning
of time, and I'm thoroughly stupefied that it hasn't been noticed before.

Here's a fix.  People who have made noises before about ties being
memory hogs should give this patch a try.

p5p-msgid: 199705172156.RAA20561@aatma.engin.umich.edu
Signed-off-by: Jay Rogers <jay@rgrs.com>
Signed-off-by: Paul Marquess <pmarquess@bfsec.bt.co.uk>

diff --git a/pp_hot.c b/pp_hot.c
index e48a010..2f044b9 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -625,13 +625,17 @@ PP(pp_aassign)
            av_extend(ary, lastrelem - relem);
            i = 0;
            while (relem <= lastrelem) {        /* gobble up all the rest */
+               SV **didstore;
                sv = NEWSV(28,0);
                assert(*relem);
                sv_setsv(sv,*relem);
                *(relem++) = sv;
-               (void)av_store(ary,i++,sv);
-               if (magic)
+               didstore = av_store(ary,i++,sv);
+               if (magic) {
                    mg_set(sv);
+                   if (!didstore)
+                       SvREFCNT_dec(sv);
+               }
                TAINT_NOT;
            }
            break;
@@ -644,6 +648,7 @@ PP(pp_aassign)
 
                while (relem < lastrelem) {     /* gobble up all the rest */
                    STRLEN len;
+                   HE *didstore;
                    if (*relem)
                        sv = *(relem++);
                    else
@@ -652,9 +657,12 @@ PP(pp_aassign)
                    if (*relem)
                        sv_setsv(tmpstr,*relem);        /* value */
                    *(relem++) = tmpstr;
-                   (void)hv_store_ent(hash,sv,tmpstr,0);
-                   if (magic)
+                   didstore = hv_store_ent(hash,sv,tmpstr,0);
+                   if (magic) {
                        mg_set(tmpstr);
+                       if (!didstore)
+                           SvREFCNT_dec(tmpstr);
+                   }
                    TAINT_NOT;
                }
                if (relem == lastrelem)