Re: [Help-smalltalk] Bug with long recursion

[Paolo Bonzini]

On 07/27/2011 11:42 AM, Paolo Bonzini wrote:
> > I've investigate a bit the long recursion bug, to debug I've choose a
> > simple recursive code:
> >
> > ObjectMemory growTo: 100 * 1024 * 1024. " This is not needed "
> > Object compile: 'foo: i [ i > 1 ifTrue: [ self foo: i - 1 ] ifFalse: [
> > #ici printNl ] ]'.
> > Object new foo: 1000000
>
> 3.2 works:

It's actually latent there.  Like all GC bugs, it's of the "how the heck 
can it work at all" kind.

The OOP that is being freed incorrectly is the Smalltalk dictionary, 
nothing less.  The incorrect free is due to the OOP being swept twice, 
the first time during the search for the first unallocated OOP, the 
second while searching for the last allocated OOP.  I haven't really 
analyzed how it happens (no time; bisection points at an unrelated 
commit), but the fix is "obvious": just use IS_OOP_VALID rather than 
IS_OOP_VALID_GC in reset_incremental_gc.

Gwen suggested offlist assigning _gst_mem.next_oop_to_sweep outside the 
#ifdef.  That is not enough, but it put me on the right track.  Thanks!

Here is the patch:

diff --git a/libgst/oop.c b/libgst/oop.c
index baa92e6..567cdb1 100644
--- a/libgst/oop.c
+++ b/libgst/oop.c
@@ -1402,19 +1402,21 @@ reset_incremental_gc (OOP firstOOP)

   _gst_mem.first_allocated_oop = oop;

-#ifdef NO_INCREMENTAL_GC
   _gst_mem.next_oop_to_sweep = _gst_mem.last_allocated_oop;
+  _gst_mem.last_swept_oop = oop - 1;
+
+#ifdef NO_INCREMENTAL_GC
   _gst_finish_incremental_gc ();
 #else
   /* Skip high OOPs that are unallocated.  */
-  for (oop = _gst_mem.last_allocated_oop; !IS_OOP_VALID_GC (oop); oop--)
+  for (oop = _gst_mem.last_allocated_oop; !IS_OOP_VALID (oop); oop--)
     _gst_sweep_oop (oop);

   _gst_mem.last_allocated_oop = oop;
   _gst_mem.next_oop_to_sweep = oop;
 #endif

-  _gst_mem.last_swept_oop = _gst_mem.first_allocated_oop - 1;
   _gst_mem.num_free_oops = _gst_mem.ot_size -
     (_gst_mem.last_allocated_oop - _gst_mem.ot);

Paolo