gc.c -- possible logic error?

[Hugh Sasse <hgs@dmu.ac.uk>]

I've been looking at Tom Copeland's memory allocation problem:

http://tomcopeland.blogs.com/juniordeveloper/2007/09/tracking-down-a.html

which I've been able to reproduce for various data structures.
This lead me to rediscover _Why's wonderful "The Fully Upturned Bin":

http://whytheluckystiff.net/articles/theFullyUpturnedBin.html

clearly GC is not releasing the memory, even if this profiler

http://code.google.com/p/ruby-memory-profiler/source

shows that ObectSpace has forgotten about this data.
That's even when I wrap the offending declarations in a
1.times {...}
block so they drop out of scope afterwards.

So I look in gc.c at how heaps are allocated, and see that if 
heaps_used is > 0 then a heaps struct is realloc'ed to be larger.
Otherwise a new struct is malloc'ed.  Now, this "otherwise" case is
when heaps_used is non-positive, i.e. zero or negative. 

Let us suppose that heaps_used is -1.  Then a new heaps struct will be 
malloc'ed this time, heaps_used then incremented to 0.  Then next time
a new heaps struct will be malloc'ed, instead of being realloc'ed as
it should.  Thus there would be two malloc calls, and one never freed.

OK, but heaps_used should never get to be negative, should it?. True, I
can't see anywhere in gc.c where it could end up less than 0.
However, why is it declared to be of type int, instead of unsigned
int?  This is the flaw I'm seeing in the logic.  I'm wondering if
heaps_length and heap_slots.limit should also be unsigned as well?
I can't see when they'd need to be negative.

The patch below is against 1.8.6-p110.  I've not tested it yet,
because I thought someone more familiar with GC than I am (probably
most of you :-)) could stop me with a good reason not to go a long
way down that road.   The patch I *haven't* posted just changes the
heaps_used to unsigned int.  That may be better and gives the
possibility of bigger heaps, perhaps.  This patch just sets it
to zero in the non-positive case, it gets incremented to 1 later.

I still haven't figured out why GC.start isn't freeing the memory in
Tom's cases, though.

        Hugh

--- ruby-1.8.6-p110/gc.c.orig   2007-03-03 07:30:46.000000000 +0000
+++ ruby-1.8.6-p110/gc.c        2007-09-28 13:30:26.344839000 +0100
@@ -335,6 +335,7 @@
            }
            else {
                p = heaps = (struct heaps_slot *)malloc(length);
+               heaps_used = 0; /* incremented later */
            });
        if (p == 0) rb_memerror();
     }