June 06, 2007
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".


-- 

November 04, 2007
http://d.puremagic.com/issues/show_bug.cgi?id=1258


bugzilla@digitalmars.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




------- Comment #1 from bugzilla@digitalmars.com  2007-11-03 21:45 -------
Fixed dmd 1.023 and 2.007


--