Psych::Emitter breaks after GC compaction: raw VALUE stored in libyaml output handler

[jeremy]

Summary

Psych::Emitter stores its own VALUE in the libyaml emitter as a raw pointer and reads it back on every write. Nothing keeps that object in place, so if GC compaction relocates the emitter between initialize and the first emit, libyaml hands back a stale address and the write handler operates on whatever object now occupies it.

Reproduction

require "psych"
require "stringio"

io = StringIO.new
# Parked off-stack on purpose: a local variable is conservatively pinned by the
# machine-stack scan, which masks the bug.
$holder = [Psych::Emitter.new(io)]

GC.verify_compaction_references(expand_heap: true, toward: :empty)

e = $holder[0]
e.start_stream(Psych::Nodes::Stream::UTF8)
e.start_document([], [], true)
e.scalar("hello", nil, nil, true, false, Psych::Nodes::Scalar::ANY)
e.end_document(true)
e.end_stream

puts io.string

Actual:

psych_emitter.rb:22:in 'Psych::Emitter#end_document': undefined method 'write' for nil (NoMethodError)

Expected (and what you get with the GC.verify_compaction_references line removed — I ran that as a control):

hello

Reproduces every time on psych 5.4.0 / ruby 4.0.6 (arm64-darwin23). The code is unchanged on master.

Cause

ext/psych/psych_emitter.c:97 hands libyaml the emitter's own VALUE:

yaml_emitter_set_output(emitter, writer, (void *)self);

and ext/psych/psych_emitter.c:27 casts it straight back on every write:

static int writer(void *ctx, unsigned char *buffer, size_t size)
{
    VALUE self = (VALUE)ctx, io = rb_attr_get(self, id_io);

psych_emitter_type has no mark function at all:

static const rb_data_type_t psych_emitter_type = {
    "Psych/emitter",
    {0, dealloc, 0,},
    ...

so nothing pins the Emitter and nothing relocates the copy of its address held inside libyaml. After compaction, self refers to some unrelated object, rb_attr_get(self, id_io) finds no @io and returns nil, and the nil.write above follows. Depending on what lands at that address the failure mode could also be a wrong result or a crash rather than a clean NoMethodError.

Scope

Narrow. Psych.dump and friends are unaffected because they construct and consume the emitter inside a single call, so it is stack-live and conservatively pinned throughout. The parser is fine too — yaml_parser_set_input(parser, io_reader, (void *)yaml) passes a stack-live local. Only a long-lived Psych::Emitter, i.e. the streaming-emission API, is exposed.

Ordinary GC.compact does not reliably reproduce it: I ran 20 rounds with allocation churn and the emitter simply never got relocated. It needs the object to actually move, which verify_compaction_references forces and which GC.auto_compact = true would make possible in the wild.

Possible fixes

Either of the two idioms already used elsewhere in the ecosystem:

Pin it, as openssl does for the VALUEs it stores in OpenSSL's ex_data (ossl_sslctx_mark calls rb_gc_mark on the value it fetched back out):

static void mark_emitter(void *ptr)
{
    yaml_emitter_t *emitter = ptr;
    if (emitter->write_handler_data) {
        rb_gc_mark((VALUE)emitter->write_handler_data);
    }
}

Or relocate it, as nokogiri does for the VALUEs it stores in libxml2's _private, with a dmark using rb_gc_mark_movable plus:

static void compact_emitter(void *ptr)
{
    yaml_emitter_t *emitter = ptr;
    if (emitter->write_handler_data) {
        emitter->write_handler_data =
            (void *)rb_gc_location((VALUE)emitter->write_handler_data);
    }
}

Happy to send a PR with whichever you prefer, plus a regression test along the lines of the reproduction above.

Context

Found while auditing this bug class after fixing the equivalent problem in sqlite3-ruby (sparklemotion/sqlite3-ruby#723, where trace and authorizer failed with the identical undefined method 'call' for nil signature for exactly the same reason).

[jeremy]

Follow-up: this is not specific to 5.4.0. I re-ran the reproducer above unchanged against
every psych I have installed, on ruby 4.0.6 (arm64-darwin23):

psych	with compaction	control (compaction line removed)
5.2.6	NoMethodError: undefined method 'write' for nil 3/3	"hello\n" 3/3
5.3.1	NoMethodError: undefined method 'write' for nil 3/3	"hello\n" 3/3
5.4.0	NoMethodError: undefined method 'write' for nil 3/3	"hello\n" 3/3

ext/psych/psych_emitter.c is byte-identical across 5.2.6, 5.3.1 and 5.4.0:

$ diff -q psych-5.2.6/ext/psych/psych_emitter.c psych-5.3.1/ext/psych/psych_emitter.c && echo IDENTICAL
IDENTICAL
$ diff -q psych-5.3.1/ext/psych/psych_emitter.c psych-5.4.0/ext/psych/psych_emitter.c && echo IDENTICAL
IDENTICAL

So the affected range is at least 5.2.6–5.4.0; 5.2.6 is just the oldest I had to hand, and
since line 97's yaml_emitter_set_output(emitter, writer, (void *)self) and the
{0, dealloc, 0,} data type at line 51 have been unchanged for years, the real range is
almost certainly wider. Worth widening the affected-version note, since 5.3.1 is what a lot of
deployments are actually running.

One other thing for whoever picks this up: main is growing a libfyaml backend
(#ifndef PSYCH_USE_LIBFYAML now wraps the file), so it'd be worth checking the fix covers
that path too.