Re: [Chicken-hackers] [PATCH] Fix some symbol GC issues

[felix . winkelmann]

> As Kooda found out when working on #1379, there's still some problems
> with the symbol GC stuff in CHICKEN 5.  Sometimes you'll see an assertion
> failure in update_symbol_tables: assert(!C_persistable_symbol(sym)) will
> bail out due to some symbols being released while they still had a global
> value or plist.
>
> Attached is a patch to fix this.  First, I added the #ifdef DEBUGBUILD
> block in update_symbol_tables() to more aggressively check for
> inconsistencies.  Then, one by one, I fixed the problems that were
> detected by this sanity check.
>
> The main problem is that originally, I thought symbols were statically
> allocated by C_h_intern, but it's only the *string* that's statically
> allocated.  Making the entire symbol statically allocated is AFAICT not
> possible (because the GC will skip its value and plist), so instead I
> changed add_symbol() to check for this and allocate a weak or regular
> pair from the start, instead of attempting to detect statically allocated
> symbols during garbage collection.  This should make GCs ever so slightly
> faster, too :)
>
> C_i_unpersist() now also needs to check for static strings, to avoid
> unpersisting those symbols prematurely.
>
> I also noticed a sort of race condition in eval.scm where it could persist
> the symbol before its value would be filled in, tripping up the sanity
> check.  I worked around that by first performing the calculation and then
> persisting.  I also removed ##sys#persist-symbol in favor of using
> a ##core#inline in eval.scm, which should also make things slightly
> faster and not susceptible to interrupts.
>
> Finally, there's a weird situation where some symbols will be allocated
> statically, but someone else might have interned the same symbol before.
> This means the existing symbol might be dynamically allocated, and it
> could be GCed, which means lf[...] will refer to dead memory (the GC can't
> update lf[...] entries).  To fix this, I decided to go ahead and allocate
> the string statically, and replace the existing symbol's dynamic string
> with the newly allocated one, and persisting the symbol to ensure it
> won't get GCed.

Excellent work, Peter. I'll try to look into this, but give me some time.

>
> I'm reasonably sure that this weird situation can also occur in master.
> I don't think it can be fixed as easily there, though, due to how the
> weak table stuff works: if nothing currently refers to the symbol, it
> will always be collected, even though the C code can refer to lf[...]
> items later.

I'd leave master as it is.


felix