From 8aac19d5987150cf5c45fee73c7a949ca472f488 Mon Sep 17 00:00:00 2001 From: Kasumi Hanazuki Date: Tue, 1 Apr 2025 19:45:37 +0000 Subject: [PATCH] io_buffer: Reimplement dcompact for IO::Buffer The `source` field in IO::Buffer can have a String or an IO::Buffer object, if not nil. - When the `source` is a String object. The `base` field points to the memory location of the String content, which can be embedded in RSTRING, and in that case, GC compaction can move the memory region along with the String object. Thus, IO::Buffer needs to pin the `source` object to prevent `base` pointer from becoming invalid. - When the `source` is an IO::Buffer, then `base` is a pointer to a malloced or mmapped memory region, managed by the source IO::Buffer. In this case, we don't need to pin the source IO::Buffer object, since the referred memory region won't get moved by GC. Closes: [Bug #21210] --- io_buffer.c | 15 ++++++++++++--- test/ruby/test_io_buffer.rb | 13 +++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/io_buffer.c b/io_buffer.c index 75b2edd475..96f13c364a 100644 --- a/io_buffer.c +++ b/io_buffer.c @@ -273,10 +273,18 @@ io_buffer_free(struct rb_io_buffer *buffer) } static void -rb_io_buffer_type_mark(void *_buffer) +rb_io_buffer_type_mark_and_move(void *_buffer) { struct rb_io_buffer *buffer = _buffer; - rb_gc_mark(buffer->source); + if (buffer->source != Qnil) { + if (RB_TYPE_P(buffer->source, T_STRING)) { + // The `source` String has to be pinned, because the `base` may point to the embedded String content, + // which can be otherwise moved by GC compaction. + rb_gc_mark(buffer->source); + } else { + rb_gc_mark_and_move(&buffer->source); + } + } } static void @@ -303,9 +311,10 @@ rb_io_buffer_type_size(const void *_buffer) static const rb_data_type_t rb_io_buffer_type = { .wrap_struct_name = "IO::Buffer", .function = { - .dmark = rb_io_buffer_type_mark, + .dmark = rb_io_buffer_type_mark_and_move, .dfree = rb_io_buffer_type_free, .dsize = rb_io_buffer_type_size, + .dcompact = rb_io_buffer_type_mark_and_move, }, .data = NULL, .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_EMBEDDABLE, diff --git a/test/ruby/test_io_buffer.rb b/test/ruby/test_io_buffer.rb index 70c5ef061d..62c4667888 100644 --- a/test/ruby/test_io_buffer.rb +++ b/test/ruby/test_io_buffer.rb @@ -693,4 +693,17 @@ class TestIOBuffer < Test::Unit::TestCase buf.set_string('a', 0, 0) assert_predicate buf, :empty? end + + # https://bugs.ruby-lang.org/issues/21210 + def test_bug_21210 + omit "compaction is not supported on this platform" unless GC.respond_to?(:compact) + + str = +"hello" + buf = IO::Buffer.for(str) + assert_predicate buf, :valid? + + GC.verify_compaction_references(expand_heap: true, toward: :empty) + + assert_predicate buf, :valid? + end end