Re: [Help-smalltalk] Compiler memory leak

[Gwenaël Casaccio]

On 16/07/2013 16:07, Paolo Bonzini wrote:
> Il 16/07/2013 09:43, Gwenaël Casaccio ha scritto:
> > Hi,
> >
> > I've found a memory leak in the compiler: there are extra
> > INC_(SAVE/RESTORE)_POINTER
> > in compile_block and _gst_make_constant that create a leak.
> Not a leak, a dangling pointer. :)
>
> > Once the object is allocated and protected in a context (ie created by
> > save_pointer)
> > once leaving the function they should be either protected or added in a
> > protected object
> > otherwise they could be GCed
> >
> > The patch remove those extra INC_(SAVE/RESTORE)_POINTER protection
> As discussed on IRC, incubator contexts should be as small as possible,
> just the time to prepare an object and pass it back to the caller.  The
> caller can then decide to incubate it, or put it in an object that is
> alive, or put it in an object that is incubated.
You know I've changed my mind, using them as a kind of global
stack for allocation could be nice and simplify the Smalltalk objects
allocation logic.
> So I think it is better to leave these INC_SAVE/RESTORE_POINTER calls in
> place.  In fact, I think those in _gst_make_constant_oop are fine.
>
> You did find a bug in compile_block.  add_literal's addition of
> blockOOP/blockClosureOOP is within compile_block's incubator context,
> which breaks because the literal can disappear as soon as compile_block
> exits.  As usual, you wonder how come we never encountered this situation.
>
> To catch more bugs like this, we need to assert that add_literal is
> called in the outermost incubator context.  We can add an
> inc_context_depth() function (which really can just return
> INC_SAVE_POINTER()), store the value in _gst_compile_method and compare
> it in add_literal.  Let me try to do a patch.
Yes but you capture only one kind of bugs tracking allocation with a
"global" incubator is easier.
> Paolo

Gwen