You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2023/01/11 06:05:04 UTC
[arrow] branch master updated: ARROW-18161: [Ruby] Refer source input in sub objects (#15217)
This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 1c97ab00ab ARROW-18161: [Ruby] Refer source input in sub objects (#15217)
1c97ab00ab is described below
commit 1c97ab00ab0c6e314b04e83737b2516fb7a5f90d
Author: Sutou Kouhei <ko...@clear-code.com>
AuthorDate: Wed Jan 11 15:04:57 2023 +0900
ARROW-18161: [Ruby] Refer source input in sub objects (#15217)
`Arrow::Table` refers the source input to prevent garbage collecting the source input. If the source input is garbage collected, `Arrow::Table` touches invalid memory. It causes a crash.
`Arrow::Table` may return a sub object (table, record batch, chunked array and array) by its method. For example, `table[0].data` returns a chunked array of the first column. If the chunked array doesn't refer the source input and table is garbage collected, the chunked array touches invalid memory. It causes a crash.
We can avoid the crash by referring the source input from sub objects too.
Authored-by: Sutou Kouhei <ko...@clear-code.com>
Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
ruby/red-arrow/lib/arrow/array.rb | 1 +
ruby/red-arrow/lib/arrow/chunked-array.rb | 12 ++++-
ruby/red-arrow/lib/arrow/column-containable.rb | 2 +-
ruby/red-arrow/lib/arrow/column.rb | 1 +
ruby/red-arrow/lib/arrow/input-referable.rb | 29 +++++++++++
ruby/red-arrow/lib/arrow/loader.rb | 1 +
ruby/red-arrow/lib/arrow/record-batch.rb | 8 ++-
ruby/red-arrow/lib/arrow/table-loader.rb | 8 +--
ruby/red-arrow/lib/arrow/table.rb | 24 ++++++---
ruby/red-arrow/test/test-table.rb | 70 ++++++++++++++++++++++++++
10 files changed, 142 insertions(+), 14 deletions(-)
diff --git a/ruby/red-arrow/lib/arrow/array.rb b/ruby/red-arrow/lib/arrow/array.rb
index c9d741a5c9..e7ca560676 100644
--- a/ruby/red-arrow/lib/arrow/array.rb
+++ b/ruby/red-arrow/lib/arrow/array.rb
@@ -22,6 +22,7 @@ module Arrow
include ArrayComputable
include GenericFilterable
include GenericTakeable
+ include InputReferable
class << self
def new(*args)
diff --git a/ruby/red-arrow/lib/arrow/chunked-array.rb b/ruby/red-arrow/lib/arrow/chunked-array.rb
index ce7308f08f..dd0fb3bfbf 100644
--- a/ruby/red-arrow/lib/arrow/chunked-array.rb
+++ b/ruby/red-arrow/lib/arrow/chunked-array.rb
@@ -22,6 +22,7 @@ module Arrow
include ArrayComputable
include GenericFilterable
include GenericTakeable
+ include InputReferable
def to_arrow
self
@@ -42,7 +43,16 @@ module Arrow
alias_method :chunks_raw, :chunks
def chunks
- @chunks ||= chunks_raw
+ @chunks ||= chunks_raw.tap do |_chunks|
+ _chunks.each do |chunk|
+ share_input(chunk)
+ end
+ end
+ end
+
+ alias_method :get_chunk_raw, :get_chunk
+ def get_chunk(i)
+ chunks[i]
end
def null?(i)
diff --git a/ruby/red-arrow/lib/arrow/column-containable.rb b/ruby/red-arrow/lib/arrow/column-containable.rb
index 002dcdb5b0..32cdb7b372 100644
--- a/ruby/red-arrow/lib/arrow/column-containable.rb
+++ b/ruby/red-arrow/lib/arrow/column-containable.rb
@@ -147,7 +147,7 @@ module Arrow
# Return column names in this object.
#
# @return [::Array<String>] column names.
- #
+ #
# @since 11.0.0
def column_names
@column_names ||= columns.collect(&:name)
diff --git a/ruby/red-arrow/lib/arrow/column.rb b/ruby/red-arrow/lib/arrow/column.rb
index 2e54bfc596..ba575381ee 100644
--- a/ruby/red-arrow/lib/arrow/column.rb
+++ b/ruby/red-arrow/lib/arrow/column.rb
@@ -27,6 +27,7 @@ module Arrow
@index = index
@field = @container.schema[@index]
@data = @container.get_column_data(@index)
+ @container.share_input(@data)
end
def name
diff --git a/ruby/red-arrow/lib/arrow/input-referable.rb b/ruby/red-arrow/lib/arrow/input-referable.rb
new file mode 100644
index 0000000000..7c2105e0b1
--- /dev/null
+++ b/ruby/red-arrow/lib/arrow/input-referable.rb
@@ -0,0 +1,29 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+module Arrow
+ module InputReferable
+ def refer_input(input)
+ @input = input
+ end
+
+ def share_input(other)
+ return unless defined?(@input)
+ other.refer_input(@input)
+ end
+ end
+end
diff --git a/ruby/red-arrow/lib/arrow/loader.rb b/ruby/red-arrow/lib/arrow/loader.rb
index 9c8300628a..71fbd9e57e 100644
--- a/ruby/red-arrow/lib/arrow/loader.rb
+++ b/ruby/red-arrow/lib/arrow/loader.rb
@@ -39,6 +39,7 @@ module Arrow
require "arrow/field-containable"
require "arrow/generic-filterable"
require "arrow/generic-takeable"
+ require "arrow/input-referable"
require "arrow/record-containable"
require "arrow/symbol-values-appendable"
diff --git a/ruby/red-arrow/lib/arrow/record-batch.rb b/ruby/red-arrow/lib/arrow/record-batch.rb
index c5aaf876b0..7a9ea296ea 100644
--- a/ruby/red-arrow/lib/arrow/record-batch.rb
+++ b/ruby/red-arrow/lib/arrow/record-batch.rb
@@ -19,9 +19,11 @@ require "arrow/raw-table-converter"
module Arrow
class RecordBatch
+ include Enumerable
+
include ColumnContainable
+ include InputReferable
include RecordContainable
- include Enumerable
class << self
def new(*args)
@@ -56,7 +58,9 @@ module Arrow
#
# @since 0.12.0
def to_table
- Table.new(schema, [self])
+ table = Table.new(schema, [self])
+ share_input(table)
+ table
end
def respond_to_missing?(name, include_private)
diff --git a/ruby/red-arrow/lib/arrow/table-loader.rb b/ruby/red-arrow/lib/arrow/table-loader.rb
index b4d68d1d50..308eb16a37 100644
--- a/ruby/red-arrow/lib/arrow/table-loader.rb
+++ b/ruby/red-arrow/lib/arrow/table-loader.rb
@@ -161,7 +161,7 @@ module Arrow
record_batches << record_batch
end
table = Table.new(schema, record_batches)
- table.instance_variable_set(:@input, input)
+ table.refer_input(input)
table
end
@@ -211,7 +211,7 @@ module Arrow
field_indexes = @options[:field_indexes]
reader.set_field_indexes(field_indexes) if field_indexes
table = reader.read_stripes
- table.instance_variable_set(:@input, input)
+ table.refer_input(input)
table
end
end
@@ -245,7 +245,7 @@ module Arrow
open_input_stream do |input|
reader = FeatherFileReader.new(input)
table = reader.read
- table.instance_variable_set(:@input, input)
+ table.refer_input(input)
table
end
end
@@ -254,7 +254,7 @@ module Arrow
open_input_stream do |input|
reader = JSONReader.new(input)
table = reader.read
- table.instance_variable_set(:@input, input)
+ table.refer_input(input)
table
end
end
diff --git a/ruby/red-arrow/lib/arrow/table.rb b/ruby/red-arrow/lib/arrow/table.rb
index cfbfe7c031..5a4c0d4670 100644
--- a/ruby/red-arrow/lib/arrow/table.rb
+++ b/ruby/red-arrow/lib/arrow/table.rb
@@ -22,6 +22,7 @@ module Arrow
include ColumnContainable
include GenericFilterable
include GenericTakeable
+ include InputReferable
include RecordContainable
class << self
@@ -188,6 +189,7 @@ module Arrow
reader = TableBatchReader.new(self)
while record_batch = reader.read_next
+ share_input(record_batch)
yield(record_batch)
end
end
@@ -346,10 +348,12 @@ module Arrow
end
end
if sliced_tables.size > 1
- sliced_tables[0].concatenate(sliced_tables[1..-1])
+ sliced_table = sliced_tables[0].concatenate(sliced_tables[1..-1])
else
- sliced_tables[0]
+ sliced_table = sliced_tables[0]
end
+ share_input(sliced_table)
+ sliced_table
end
# TODO
@@ -401,7 +405,9 @@ module Arrow
new_fields << new_column[:field]
new_arrays << new_column[:data]
end
- self.class.new(new_fields, new_arrays)
+ table = self.class.new(new_fields, new_arrays)
+ share_input(table)
+ table
end
alias_method :remove_column_raw, :remove_column
@@ -423,7 +429,9 @@ module Arrow
raise IndexError.new(message)
end
end
- remove_column_raw(index)
+ table = remove_column_raw(index)
+ share_input(table)
+ table
end
# Experimental
@@ -445,7 +453,9 @@ module Arrow
packed_arrays = columns.collect do |column|
column.data.pack
end
- self.class.new(schema, packed_arrays)
+ table = self.class.new(schema, packed_arrays)
+ share_input(table)
+ table
end
# Join another Table by matching with keys.
@@ -538,7 +548,9 @@ module Arrow
plan.start
plan.wait
reader = sink_node_options.get_reader(hash_join_node.output_schema)
- reader.read_all
+ table = reader.read_all
+ share_input(table)
+ table
end
alias_method :to_s_raw, :to_s
diff --git a/ruby/red-arrow/test/test-table.rb b/ruby/red-arrow/test/test-table.rb
index 223393dd12..dace8164f6 100644
--- a/ruby/red-arrow/test/test-table.rb
+++ b/ruby/red-arrow/test/test-table.rb
@@ -860,6 +860,76 @@ chris\t-1
end
end
end
+
+ sub_test_case("GC") do
+ def setup
+ table = Arrow::Table.new(integer: [1, 2, 3],
+ string: ["a", "b", "c"])
+ @buffer = Arrow::ResizableBuffer.new(1024)
+ table.save(@buffer, format: :arrow)
+ @loaded_table = Arrow::Table.load(@buffer)
+ end
+
+ def test_chunked_array
+ chunked_array = @loaded_table[0].data
+ assert_equal(@buffer,
+ chunked_array.instance_variable_get(:@input).buffer)
+ end
+
+ def test_array
+ array = @loaded_table[0].data.chunks[0]
+ assert_equal(@buffer,
+ array.instance_variable_get(:@input).buffer)
+ end
+
+ def test_record_batch
+ record_batch = @loaded_table.each_record_batch.first
+ assert_equal(@buffer,
+ record_batch.instance_variable_get(:@input).buffer)
+ end
+
+ def test_record_batch_array
+ array = @loaded_table.each_record_batch.first[0].data
+ assert_equal(@buffer,
+ array.instance_variable_get(:@input).buffer)
+ end
+
+ def test_record_batch_table
+ table = @loaded_table.each_record_batch.first.to_table
+ assert_equal(@buffer,
+ table.instance_variable_get(:@input).buffer)
+ end
+
+ def test_slice
+ table = @loaded_table.slice(0..-1)
+ assert_equal(@buffer,
+ table.instance_variable_get(:@input).buffer)
+ end
+
+ def test_merge
+ table = @loaded_table.merge({})
+ assert_equal(@buffer,
+ table.instance_variable_get(:@input).buffer)
+ end
+
+ def test_remove_column
+ table = @loaded_table.remove_column(0)
+ assert_equal(@buffer,
+ table.instance_variable_get(:@input).buffer)
+ end
+
+ def test_pack
+ table = @loaded_table.pack
+ assert_equal(@buffer,
+ table.instance_variable_get(:@input).buffer)
+ end
+
+ def test_join
+ table = @loaded_table.join(@loaded_table, :integer)
+ assert_equal(@buffer,
+ table.instance_variable_get(:@input).buffer)
+ end
+ end
end
test("#pack") do