Copilot commented on code in PR #55376:
URL: https://github.com/apache/arrow/pull/55376#discussion_r4453771290
##########
ruby/red-arrow/ext/arrow/arrow.cpp:
##########
@@ -44,6 +44,38 @@ namespace red_arrow {
VALUE nanosecond;
}
+ void
+ array_mark(gpointer object)
+ {
+ const char *property_names[] = {
+ "null-bitmap",
+ "buffer1",
+ "buffer2",
+ "parent",
Review Comment:
The callback skips child arrays retained by composite types, including list
`raw-values`, dictionary `indices`/`dictionary`, and struct/union fields. A
chunk containing these arrays can therefore still lose a child's String-backed
buffer during GC and read freed memory. Mark these retained children through
non-allocating accessors and add a nested-array GC regression.
##########
ruby/red-arrow/ext/arrow/arrow.cpp:
##########
@@ -150,6 +182,10 @@ extern "C" void Init_arrow() {
red_arrow::symbols::month = ID2SYM(rb_intern("month"));
red_arrow::symbols::nanosecond = ID2SYM(rb_intern("nanosecond"));
+ rbgobj_register_mark_func(GARROW_TYPE_ARRAY,
+ red_arrow::array_mark);
Review Comment:
An array built from `Arrow::Buffer.new(data).slice(...)` can still lose its
backing String during GC: marking the sliced buffer does not mark its native
`parent`. Register a buffer mark callback that follows that parent reference,
and add a sliced-buffer GC regression.
##########
ruby/red-arrow/ext/arrow/arrow.cpp:
##########
@@ -150,6 +182,10 @@ extern "C" void Init_arrow() {
red_arrow::symbols::month = ID2SYM(rb_intern("month"));
red_arrow::symbols::nanosecond = ID2SYM(rb_intern("nanosecond"));
+ rbgobj_register_mark_func(GARROW_TYPE_ARRAY,
+ red_arrow::array_mark);
+ rbgobj_register_mark_func(GARROW_TYPE_CHUNKED_ARRAY,
+ red_arrow::chunked_array_mark);
Review Comment:
The added tests only exercise GLib chunk caching; `c_glib/test/run-test.rb`
loads Arrow through `GI.load`, so it never loads these Ruby mark callbacks.
Both callbacks could be removed without failing those tests. Please add a
regression in `ruby/red-arrow/test/test-chunked-array.rb` that constructs a
chunked array from temporary String-backed buffers, forces GC before reading
any chunks, and verifies that the buffers remain alive and the values are
unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
