Skip to content

types: give the collector a dedicated mark bit - #488

Merged
jow- merged 2 commits into
masterfrom
fix/gc-dedicated-mark-bit
Oct 16, 2026
Merged

jow- merged 2 commits into
masterfrom
fix/gc-dedicated-mark-bit

Conversation

@jow-

@jow- jow- commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Counter-proposal to #477, which fixes the same bug with a different mechanism.

The bug

ucv_to_stringbuf_formatted() uses the single mark bit in the value header
for stringification cycle detection, but the collector's reachability mark and
the sweep use that very same bit. A tostring converter runs arbitrary script
code and may trigger a collection; with the traversal path already marked, the
mark phase stops at the marked container, its not-yet-formatted children are
swept, and the sweep clears the mark — so the cycle detection silently stops
working for the rest of the traversal and cycles print as "null".

$ ucode -p 'let a; a = [ proto({}, { tostring: () => { gc(); return "A"; } }), { b: 7 } ]; a'
[
    "A",
    "null"      # <- should be { "b": 7 }
]

Approach

Give the collector its own mark bit so the two markings can never collide, and
make the stringification path self-contained so the collector stays unaware of
it:

  • Dedicated gc_mark bit. The header's 26-bit refcount field lends its
    top bit to the collector, reducing it to 25 bits. The field is declared so
    that this top bit is taken on both little- and big-endian, so type, mark
    and ext_flag keep their exact bit positions and only a refcount over 33
    million would ever set the bit. Modules reaching the refcount through the
    supported ucv_get() / ucv_put() API are unaffected.

  • Self-contained stringification. The value being formatted is rooted on
    the VM value stack for the duration of the traversal, so the collector keeps
    it (and everything it references) alive across callback collections; and the
    container is made constant while it is traversed so that a converter cannot
    add, remove or replace its entries mid-traversal.

Because the collector now uses its own bit, its mark phase always descends into
containers under stringification and its sweep never touches their cycle
detection marks. Explicit gc() and automatic collections keep working from
inside converters, with no per-container bookkeeping in the thread context and
no mark clear/restore passes in ucv_gc_common().

Why this over #477

  • No GC hook. The collector does not need to know about the formatting
    state at all, so the interaction cannot regress when the GC changes.
  • One bit instead of a C-stack linked list. The alternative threads a
    per-container frame through the C stack, walks it on every collection, and
    must unwind it at every formatting exit point (including goto out and an
    exit() in a converter). The bit needs no bookkeeping and no unwind.
  • Rooted for the whole traversal, including re-entrant formatting, not only
    across the direct converter dispatch.

Testing

  • New regression test tests/custom/99_bugs/59_format_gc_live_values covers
    explicit and automatic collection, arrays and objects, re-entrant formatting
    with cycles and shared values, rejected mutations of containers under
    stringification, converter exceptions, exit() in a converter, converters
    modifying their own value, formatting of constant values, and collection of
    unreachable cycles in a callback.
  • Full custom suite passes (252 tests) under a normal build and an ASan/UBSan
    build.

The stringification cycle detection, the collector's reachability walk
and ucv_free()'s reentrancy guard all share the single mark bit in the
value header.  A tostring callback executes arbitrary script code and
may trigger a collection while the formatter has marked the active
traversal path: the mark phase stops at the already marked container,
its not-yet-formatted children are swept and the sweep clears the
mark, defeating the cycle detection for the rest of the traversal.

Push a reference to the value being stringified onto the VM stack to
keep it reachable across callbacks, popping it again on the way out
unless exit() has already reset the stack, and mark the container
constant for the duration of its traversal so that callbacks cannot
add, remove or replace its entries, restoring the original constness
afterwards.

Separate the collector's reachability mark into its own header bit so
that the mark phase always descends into containers under
stringification and the sweep never touches their cycle detection
marks.  The new bit reuses the top bit of the former 26 bit refcount
field, reducing the refcount to 25 bits; the field is declared so that
this top bit is taken on both endiannesses, and only a refcount above 33
million would ever have set it, so no realistic value is affected,
`type`, `mark` and `ext_flag` keep their positions, and modules
reaching the refcount through ucv_get()/ucv_put() are unaffected.

This removes the need for the collector to be aware of the formatting
state at all: no per-container bookkeeping in the thread context and no
mark clear/restore passes in ucv_gc_common() are required, while
explicit and automatic collection keep working from inside converters.

Add regressions for explicit and automatic collection, arrays and
objects, reentrant formatting with cycles and shared values, rejected
mutations of containers under stringification, converter exceptions,
exit() in a converter, converters modifying their own value, formatting
of constant values and collection of unreachable cycles in a callback.

Fixes `ucode -p 'let a; a = [ proto({}, { tostring: () => { gc();
return "A"; } }), { b: 7 } ]; a'` corrupting the output.
@darekhta

Copy link
Copy Markdown

Thanks, I like this approach better. Giving the GC its own mark bit is a cleaner solution.

I found one related corner case that affects both versions:

print(proto({}, { tostring: () => exit(3) }));

exit() drops the stack reference and can free the value before the formatter resumes. I’ve attached a small follow-up patch to handle this, along with a regression test.

I’ll close #477 in favor of this one. Thanks for taking the time to work through this!
0001-types-avoid-accessing-values-after-converter-exit.patch

An exit() in a tostring converter resets the VM stack, dropping the
formatter's root. A directly formatted temporary value may be freed before
ucv_call_tostring() returns false, so falling back to the default traversal
accesses freed memory.

Check for EXCEPTION_EXIT after converter dispatch and leave through the
existing cleanup path before constifying or traversing the value.

Add a regression for a directly formatted temporary object. Use sprintf()
in the nested exit regression so it checks the exit status and skipped
converters without depending on partially emitted output.

Suggested-by: darekhta <dmitri.arekhta@me.com>
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
@jow-
jow- merged commit 20eb2ca into master Oct 16, 2026
3 checks passed
@jow-
jow- deleted the fix/gc-dedicated-mark-bit branch October 16, 2026 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants