chicken-hackers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Bex
Subject: [Chicken-hackers] [PATCH] Fix some symbol GC issues
Date: Fri, 16 Jun 2017 21:29:51 +0200
User-agent: Mutt/1.5.23 (2014-03-12)

Hi all,

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.

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.

Cheers,
Peter

Attachment: 0001-Fix-some-edge-cases-with-symbol-GC.patch
Description: Text Data

Attachment: signature.asc
Description: Digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]