Skip to content

Commit 8aac19d

Browse files
hanazukibyroot
authored andcommitted
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]
1 parent 4cc58c3 commit 8aac19d

2 files changed

Lines changed: 25 additions & 3 deletions

File tree

io_buffer.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,10 +273,18 @@ io_buffer_free(struct rb_io_buffer *buffer)
273273
}
274274

275275
static void
276-
rb_io_buffer_type_mark(void *_buffer)
276+
rb_io_buffer_type_mark_and_move(void *_buffer)
277277
{
278278
struct rb_io_buffer *buffer = _buffer;
279-
rb_gc_mark(buffer->source);
279+
if (buffer->source != Qnil) {
280+
if (RB_TYPE_P(buffer->source, T_STRING)) {
281+
// The `source` String has to be pinned, because the `base` may point to the embedded String content,
282+
// which can be otherwise moved by GC compaction.
283+
rb_gc_mark(buffer->source);
284+
} else {
285+
rb_gc_mark_and_move(&buffer->source);
286+
}
287+
}
280288
}
281289

282290
static void
@@ -303,9 +311,10 @@ rb_io_buffer_type_size(const void *_buffer)
303311
static const rb_data_type_t rb_io_buffer_type = {
304312
.wrap_struct_name = "IO::Buffer",
305313
.function = {
306-
.dmark = rb_io_buffer_type_mark,
314+
.dmark = rb_io_buffer_type_mark_and_move,
307315
.dfree = rb_io_buffer_type_free,
308316
.dsize = rb_io_buffer_type_size,
317+
.dcompact = rb_io_buffer_type_mark_and_move,
309318
},
310319
.data = NULL,
311320
.flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_EMBEDDABLE,

test/ruby/test_io_buffer.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,4 +693,17 @@ def test_set_string_null_destination
693693
buf.set_string('a', 0, 0)
694694
assert_predicate buf, :empty?
695695
end
696+
697+
# https://bugs.ruby-lang.org/issues/21210
698+
def test_bug_21210
699+
omit "compaction is not supported on this platform" unless GC.respond_to?(:compact)
700+
701+
str = +"hello"
702+
buf = IO::Buffer.for(str)
703+
assert_predicate buf, :valid?
704+
705+
GC.verify_compaction_references(expand_heap: true, toward: :empty)
706+
707+
assert_predicate buf, :valid?
708+
end
696709
end

0 commit comments

Comments
 (0)