http://d.puremagic.com/issues/show_bug.cgi?id=1258

           Summary: Garbage collector loses memory upon array concatenation
           Product: D
           Version: 1.015
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Keywords: patch
          Severity: critical
          Priority: P2
         Component: Phobos
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: babele.dunnit@gmail.com


GC is unable to re-collect memory from locally-created and concatenated dynamic
arrays. Will crash the system if left running. No problem with array slicing.
This snippet will demonstrate the problem:

----------------------------------------------------------------------------------------------------------


import std.stdio;

class Individual
{
    Individual[20] children;
    // substitute the array hereabove with
    // real[20] someReals
    // and GC will work OK
}

class Population
{

    void grow()
    {
        foreach(inout individual; individuals)
        {
            individual = new Individual;
        }
    }

    Individual[20000] individuals;
}

// change this and it will work flawlessly
version = loseMemory;

int main(char[][] args)
{

    Population testPop1 = new Population;
    Population testPop2 = new Population;

    Individual[40000] indi;

    for(int i = 0; i < 1000000; i++)
    {
        writefln("Round %d", i);

        testPop1.grow();
        testPop2.grow();

        version (loseMemory){
            indi[] = testPop1.individuals ~ testPop2.individuals;
        }

        version (everythingOk){
            indi[0..20000] = testPop1.individuals;
            indi[20000..40000] = testPop2.individuals;}
        }

    return 0;
}

-------------------------------------------------------------------------------------------------------------------

Frits van Bommel says:

-----------------------

Looking at the GC code I can't seem to find any place where arr[length
.. _gc.cap(arr)] (the unused part of the array allocation) is
initialized. This could explain the issue if your arrays have different
lengths (since the data from an longer old array may be present after a
shorter new array, and is considered as "live" pointers by the GC
because it's within the same allocation block).

However, this seems to be the case for straight allocation as well, not
just concatenation.
If this is the cause, you probably have the same issue if you replace
     indi[] = testPop1.individuals ~ testPop2.individuals;
by
     auto tmp = new Individual[](testPop1.length + testPop2.length);
     tmp[0 .. testPop1.length] = testPop1;
     tmp[testPop1.length .. $] = testPop2;
     indi[] = tmp;

----------------------

and I confirm Frits hypotesis; moreover, Oskar Linde says:

----------------------

The code that clears arr[length.._gc.cap(arr)] lies in gcx.d. Search for
the phrase "inline"

The code is actually commented out on DMD:
//foreach(inout byte b; cast(byte[])(p + size)[0..binsize[bin] - size])
{ b = 0; }

A patch that reverses that comment:

--- gcx.d       2007-06-04 16:47:02.354590379 +0200
+++ gcx.d.new   2007-06-04 16:46:53.331933006 +0200
@@ -297,7 +297,7 @@
                 gcx.bucket[bin] = (cast(List *)p).next;
                 //memset(p + size, 0, binsize[bin] - size);
                 // 'inline' memset - Dave Fladebo.
-               //foreach(inout byte b; cast(byte[])(p +
size)[0..binsize[bin] - size]) { b = 0; }
+               foreach(inout byte b; cast(byte[])(p +
size)[0..binsize[bin] - size]) { b = 0; }
                 //debug(PRINTF) printf("\tmalloc => %x\n", p);
                 debug (MEMSTOMP) memset(p, 0xF0, size);
             }

-----------------------

and I confirm this patch fixes the problem under Windows.

The leak is present also with much smaller arrays, even if in that case will
probably not waste enough memory to crash your system in a reasonably short
time.

Please note: TODAY (07/06/06) Walter Bright released DMD 1.015 and the
changelog says "Fixed gc memory corrupting problem". I was hoping Walter
referred to this bug, but this is not the case; the problem is still there and
I had to re-patch Phobos.

Also: why was that zero-ing commented out? seems to be the "right thing" to
do...

I marked this out as "critical"; as David B. Held says, "Looks pretty
important" and this is a "crash" and a "severe memory leak".


--