This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 463682df3b GH-50302: [GLib][Ruby][FlightRPC] Fix GC related problems
(#50401)
463682df3b is described below
commit 463682df3b4e6209b92dc8fdd46460e16d0b6fa6
Author: Sutou Kouhei <[email protected]>
AuthorDate: Thu Jul 9 09:38:55 2026 +0900
GH-50302: [GLib][Ruby][FlightRPC] Fix GC related problems (#50401)
### Rationale for this change
`gaflight_server_listen()` may use options' auth handler later. So we
should refer the given options while the server is alive. If we don't refer the
given options, we may touch freed auth handler later.
The argument of `ArrowFlight::Criteria.new`/`ArrowFlight::Ticket.new`
should be referred to protect from GC in Ruby.
I could fix some problems by `GC.stress = true` but I couldn't fix some
problems that are reproduced only on GitHub Actions. I omit tests for them for
now.
### What changes are included in this PR?
* Refer options for `gaflight_server_listen()`
* Refer expression for `ArrowFlight::Criteria.new`
* Refer data for `ArrowFlight::Ticket.new`
* Omit Flight related tests that I can't fix...
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* GitHub Issue: #50302
Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
c_glib/arrow-flight-glib/server.cpp | 61 ++++++++++++++++++++++
c_glib/test/flight/test-client.rb | 5 +-
c_glib/test/flight/test-criteria.rb | 3 +-
c_glib/test/flight/test-endpoint.rb | 12 +++--
c_glib/test/flight/test-ticket.rb | 15 ++++--
ci/scripts/c_glib_test.sh | 6 ++-
ruby/red-arrow-cuda/Rakefile | 4 +-
ruby/red-arrow-dataset/Rakefile | 4 +-
ruby/red-arrow-flight-sql/Rakefile | 4 +-
ruby/red-arrow-flight-sql/test/test-client.rb | 2 +-
ruby/red-arrow-flight/Rakefile | 4 +-
.../lib/arrow-flight/{ticket.rb => criteria.rb} | 11 +++-
ruby/red-arrow-flight/lib/arrow-flight/loader.rb | 1 +
ruby/red-arrow-flight/lib/arrow-flight/ticket.rb | 8 +++
.../ticket.rb => test/test-criteria.rb} | 19 +++----
.../test/test-record-batch-reader.rb | 1 +
ruby/red-arrow-format/Rakefile | 4 +-
ruby/red-arrow/Rakefile | 4 +-
ruby/red-gandiva/Rakefile | 4 +-
ruby/red-parquet/Rakefile | 4 +-
20 files changed, 140 insertions(+), 36 deletions(-)
diff --git a/c_glib/arrow-flight-glib/server.cpp
b/c_glib/arrow-flight-glib/server.cpp
index 2feeb853e2..e354c70624 100644
--- a/c_glib/arrow-flight-glib/server.cpp
+++ b/c_glib/arrow-flight-glib/server.cpp
@@ -1166,6 +1166,7 @@ G_BEGIN_DECLS
struct GAFlightServerPrivate
{
gaflight::Server server;
+ GAFlightServerOptions *options;
};
G_END_DECLS
@@ -1179,6 +1180,10 @@
gaflight_server_servable_interface_init(GAFlightServableInterface *iface)
iface->get_raw = gaflight_server_servable_get_raw;
}
+enum {
+ PROP_OPTIONS = 1,
+};
+
G_DEFINE_ABSTRACT_TYPE_WITH_CODE(
GAFlightServer, gaflight_server, G_TYPE_OBJECT,
G_ADD_PRIVATE(GAFlightServer);
G_IMPLEMENT_INTERFACE(GAFLIGHT_TYPE_SERVABLE,
gaflight_server_servable_interface_init))
@@ -1196,6 +1201,19 @@ gaflight_server_servable_get_raw(GAFlightServable
*servable)
}
G_BEGIN_DECLS
+static void
+gaflight_server_dispose(GObject *object)
+{
+ auto priv = GAFLIGHT_SERVER_GET_PRIVATE(object);
+
+ if (priv->options) {
+ g_object_unref(priv->options);
+ priv->options = nullptr;
+ }
+
+ G_OBJECT_CLASS(gaflight_server_parent_class)->dispose(object);
+}
+
static void
gaflight_server_finalize(GObject *object)
{
@@ -1205,6 +1223,24 @@ gaflight_server_finalize(GObject *object)
G_OBJECT_CLASS(gaflight_server_parent_class)->finalize(object);
}
+static void
+gaflight_server_get_property(GObject *object,
+ guint prop_id,
+ GValue *value,
+ GParamSpec *pspec)
+{
+ auto priv = GAFLIGHT_SERVER_GET_PRIVATE(object);
+
+ switch (prop_id) {
+ case PROP_OPTIONS:
+ g_value_set_object(value, priv->options);
+ break;
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+ break;
+ }
+}
+
static void
gaflight_server_init(GAFlightServer *object)
{
@@ -1216,7 +1252,24 @@ static void
gaflight_server_class_init(GAFlightServerClass *klass)
{
auto gobject_class = G_OBJECT_CLASS(klass);
+ gobject_class->dispose = gaflight_server_dispose;
gobject_class->finalize = gaflight_server_finalize;
+ gobject_class->get_property = gaflight_server_get_property;
+
+ GParamSpec *spec;
+ /**
+ * GAFlightServer:options:
+ *
+ * The options used in this server.
+ *
+ * Since: 26.0.0
+ */
+ spec = g_param_spec_object("options",
+ "Options",
+ "The options used in this server",
+ GAFLIGHT_TYPE_SERVER_OPTIONS,
+ static_cast<GParamFlags>(G_PARAM_READABLE));
+ g_object_class_install_property(gobject_class, PROP_OPTIONS, spec);
}
/**
@@ -1236,6 +1289,14 @@ gaflight_server_listen(GAFlightServer *server,
{
auto flight_server = gaflight_servable_get_raw(GAFLIGHT_SERVABLE(server));
const auto flight_options = gaflight_server_options_get_raw(options);
+ auto priv = GAFLIGHT_SERVER_GET_PRIVATE(server);
+ if (priv->options != options) {
+ g_object_ref(options);
+ if (priv->options) {
+ g_object_unref(priv->options);
+ }
+ priv->options = options;
+ }
return garrow::check(error,
flight_server->Init(*flight_options),
"[flight-server][listen]");
diff --git a/c_glib/test/flight/test-client.rb
b/c_glib/test/flight/test-client.rb
index fd7bd25a19..f325d05da0 100644
--- a/c_glib/test/flight/test-client.rb
+++ b/c_glib/test/flight/test-client.rb
@@ -22,7 +22,7 @@ class TestFlightClient < Test::Unit::TestCase
@server = nil
omit("Arrow Flight is required") unless defined?(ArrowFlight)
omit("Unstable on Windows") if Gem.win_platform?
- omit("Unstable on x86_64 macOS") if /x86_64-darwin/.match?(RUBY_PLATFORM)
+ omit("Unstable on macOS") if /darwin/.match?(RUBY_PLATFORM)
require_gi_bindings(3, 4, 7)
@server = Helper::FlightServer.new
host = "127.0.0.1"
@@ -80,8 +80,9 @@ class TestFlightClient < Test::Unit::TestCase
def test_error
client = ArrowFlight::Client.new(@location)
+ invalid_data = GLib::Bytes.new("invalid")
assert_raise(Arrow::Error::Invalid) do
- client.do_get(ArrowFlight::Ticket.new("invalid"))
+ client.do_get(ArrowFlight::Ticket.new(invalid_data))
end
end
end
diff --git a/c_glib/test/flight/test-criteria.rb
b/c_glib/test/flight/test-criteria.rb
index d5f60a8953..c2e66369d2 100644
--- a/c_glib/test/flight/test-criteria.rb
+++ b/c_glib/test/flight/test-criteria.rb
@@ -22,7 +22,8 @@ class TestFlightCriteria < Test::Unit::TestCase
def test_expression
expression = "expression"
- criteria = ArrowFlight::Criteria.new(expression)
+ expression_bytes = GLib::Bytes.new(expression)
+ criteria = ArrowFlight::Criteria.new(expression_bytes)
assert_equal(expression,
criteria.expression.to_s)
end
diff --git a/c_glib/test/flight/test-endpoint.rb
b/c_glib/test/flight/test-endpoint.rb
index 06cddf0019..0766cac325 100644
--- a/c_glib/test/flight/test-endpoint.rb
+++ b/c_glib/test/flight/test-endpoint.rb
@@ -21,7 +21,8 @@ class TestFlightEndpoint < Test::Unit::TestCase
end
def test_ticket
- ticket = ArrowFlight::Ticket.new("data")
+ data = GLib::Bytes.new("data")
+ ticket = ArrowFlight::Ticket.new(data)
locations = [
ArrowFlight::Location.new("grpc://127.0.0.1:2929"),
ArrowFlight::Location.new("grpc+tcp://127.0.0.1:12929"),
@@ -32,7 +33,8 @@ class TestFlightEndpoint < Test::Unit::TestCase
end
def test_locations
- ticket = ArrowFlight::Ticket.new("data")
+ data = GLib::Bytes.new("data")
+ ticket = ArrowFlight::Ticket.new(data)
locations = [
ArrowFlight::Location.new("grpc://127.0.0.1:2929"),
ArrowFlight::Location.new("grpc+tcp://127.0.0.1:12929"),
@@ -44,7 +46,8 @@ class TestFlightEndpoint < Test::Unit::TestCase
sub_test_case("#==") do
def test_true
- ticket = ArrowFlight::Ticket.new("data")
+ data = GLib::Bytes.new("data")
+ ticket = ArrowFlight::Ticket.new(data)
location = ArrowFlight::Location.new("grpc://127.0.0.1:2929")
endpoint1 = ArrowFlight::Endpoint.new(ticket, [location])
endpoint2 = ArrowFlight::Endpoint.new(ticket, [location])
@@ -54,7 +57,8 @@ class TestFlightEndpoint < Test::Unit::TestCase
end
def test_false
- ticket = ArrowFlight::Ticket.new("data")
+ data = GLib::Bytes.new("data")
+ ticket = ArrowFlight::Ticket.new(data)
location1 = ArrowFlight::Location.new("grpc://127.0.0.1:2929")
location2 = ArrowFlight::Location.new("grpc://127.0.0.1:1129")
endpoint1 = ArrowFlight::Endpoint.new(ticket, [location1])
diff --git a/c_glib/test/flight/test-ticket.rb
b/c_glib/test/flight/test-ticket.rb
index 976089762f..b1b0c0fb4c 100644
--- a/c_glib/test/flight/test-ticket.rb
+++ b/c_glib/test/flight/test-ticket.rb
@@ -22,23 +22,28 @@ class TestFlightTicket < Test::Unit::TestCase
def test_data
data = "data"
- ticket = ArrowFlight::Ticket.new(data)
+ data_bytes = GLib::Bytes.new(data)
+ ticket = ArrowFlight::Ticket.new(data_bytes)
assert_equal(data,
ticket.data.to_s)
end
sub_test_case("#==") do
def test_true
- ticket1 = ArrowFlight::Ticket.new("data")
- ticket2 = ArrowFlight::Ticket.new("data")
+ data1 = GLib::Bytes.new("data")
+ data2 = GLib::Bytes.new("data")
+ ticket1 = ArrowFlight::Ticket.new(data1)
+ ticket2 = ArrowFlight::Ticket.new(data2)
assert do
ticket1 == ticket2
end
end
def test_false
- ticket1 = ArrowFlight::Ticket.new("data1")
- ticket2 = ArrowFlight::Ticket.new("data2")
+ data1 = GLib::Bytes.new("data1")
+ data2 = GLib::Bytes.new("data2")
+ ticket1 = ArrowFlight::Ticket.new(data1)
+ ticket2 = ArrowFlight::Ticket.new(data2)
assert do
not (ticket1 == ticket2)
end
diff --git a/ci/scripts/c_glib_test.sh b/ci/scripts/c_glib_test.sh
index 19131709e2..dd30ebb2f4 100755
--- a/ci/scripts/c_glib_test.sh
+++ b/ci/scripts/c_glib_test.sh
@@ -36,7 +36,11 @@ fi
pushd "${source_dir}"
-ruby test/run-test.rb
+run_test_args=()
+if [ -n "${RUNNER_DEBUG}" ]; then
+ run_test_args+=(-v)
+fi
+ruby test/run-test.rb "${run_test_args[@]}"
if [[ "$(uname -s)" == "Linux" ]]; then
# TODO(kszucs): on osx it fails to load 'lgi.corelgilua51' despite that lgi
diff --git a/ruby/red-arrow-cuda/Rakefile b/ruby/red-arrow-cuda/Rakefile
index 2bbe6e7619..77c9f81e99 100644
--- a/ruby/red-arrow-cuda/Rakefile
+++ b/ruby/red-arrow-cuda/Rakefile
@@ -34,7 +34,9 @@ task :test do
cd("dependency-check") do
ruby("-S", "rake")
end
- ruby("test/run-test.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run-test.rb", *args)
end
end
diff --git a/ruby/red-arrow-dataset/Rakefile b/ruby/red-arrow-dataset/Rakefile
index 2bbe6e7619..77c9f81e99 100644
--- a/ruby/red-arrow-dataset/Rakefile
+++ b/ruby/red-arrow-dataset/Rakefile
@@ -34,7 +34,9 @@ task :test do
cd("dependency-check") do
ruby("-S", "rake")
end
- ruby("test/run-test.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run-test.rb", *args)
end
end
diff --git a/ruby/red-arrow-flight-sql/Rakefile
b/ruby/red-arrow-flight-sql/Rakefile
index 2bbe6e7619..77c9f81e99 100644
--- a/ruby/red-arrow-flight-sql/Rakefile
+++ b/ruby/red-arrow-flight-sql/Rakefile
@@ -34,7 +34,9 @@ task :test do
cd("dependency-check") do
ruby("-S", "rake")
end
- ruby("test/run-test.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run-test.rb", *args)
end
end
diff --git a/ruby/red-arrow-flight-sql/test/test-client.rb
b/ruby/red-arrow-flight-sql/test/test-client.rb
index 1c77e4d98a..a1fc366f05 100644
--- a/ruby/red-arrow-flight-sql/test/test-client.rb
+++ b/ruby/red-arrow-flight-sql/test/test-client.rb
@@ -19,7 +19,7 @@ class TestClient < Test::Unit::TestCase
def setup
@server = nil
omit("Unstable on Windows") if Gem.win_platform?
- omit("Unstable on x86_64 macOS") if /x86_64-darwin/.match?(RUBY_PLATFORM)
+ omit("Unstable on macOS") if /darwin/.match?(RUBY_PLATFORM)
@server = Helper::Server.new
@server.listen("grpc://127.0.0.1:0")
@location = "grpc://127.0.0.1:#{@server.port}"
diff --git a/ruby/red-arrow-flight/Rakefile b/ruby/red-arrow-flight/Rakefile
index 2bbe6e7619..77c9f81e99 100644
--- a/ruby/red-arrow-flight/Rakefile
+++ b/ruby/red-arrow-flight/Rakefile
@@ -34,7 +34,9 @@ task :test do
cd("dependency-check") do
ruby("-S", "rake")
end
- ruby("test/run-test.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run-test.rb", *args)
end
end
diff --git a/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
b/ruby/red-arrow-flight/lib/arrow-flight/criteria.rb
similarity index 78%
copy from ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
copy to ruby/red-arrow-flight/lib/arrow-flight/criteria.rb
index f65f3d6cfe..268a629424 100644
--- a/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
+++ b/ruby/red-arrow-flight/lib/arrow-flight/criteria.rb
@@ -16,9 +16,8 @@
# under the License.
module ArrowFlight
- class Ticket
+ class Criteria
class << self
- # @api private
def try_convert(value)
case value
when String, GLib::Bytes
@@ -28,5 +27,13 @@ module ArrowFlight
end
end
end
+
+ alias_method :initialize_raw, :initialize
+ private :initialize_raw
+ def initialize(expression)
+ expression = GLib::Bytes.new(expression) if expression.is_a?(String)
+ initialize_raw(expression)
+ @expression = expression
+ end
end
end
diff --git a/ruby/red-arrow-flight/lib/arrow-flight/loader.rb
b/ruby/red-arrow-flight/lib/arrow-flight/loader.rb
index e9e9e08f32..ed9fb09c77 100644
--- a/ruby/red-arrow-flight/lib/arrow-flight/loader.rb
+++ b/ruby/red-arrow-flight/lib/arrow-flight/loader.rb
@@ -32,6 +32,7 @@ module ArrowFlight
require "arrow-flight/call-options"
require "arrow-flight/client"
require "arrow-flight/client-options"
+ require "arrow-flight/criteria"
require "arrow-flight/location"
require "arrow-flight/record-batch-reader"
require "arrow-flight/server-call-context"
diff --git a/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
b/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
index f65f3d6cfe..c1c23ca050 100644
--- a/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
+++ b/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
@@ -28,5 +28,13 @@ module ArrowFlight
end
end
end
+
+ alias_method :initialize_raw, :initialize
+ private :initialize_raw
+ def initialize(data)
+ data = GLib::Bytes.new(data) if data.is_a?(String)
+ initialize_raw(data)
+ @data = data
+ end
end
end
diff --git a/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
b/ruby/red-arrow-flight/test/test-criteria.rb
similarity index 75%
copy from ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
copy to ruby/red-arrow-flight/test/test-criteria.rb
index f65f3d6cfe..4775d1723e 100644
--- a/ruby/red-arrow-flight/lib/arrow-flight/ticket.rb
+++ b/ruby/red-arrow-flight/test/test-criteria.rb
@@ -15,18 +15,13 @@
# specific language governing permissions and limitations
# under the License.
-module ArrowFlight
- class Ticket
- class << self
- # @api private
- def try_convert(value)
- case value
- when String, GLib::Bytes
- new(value)
- else
- nil
- end
- end
+class TestCriteria < Test::Unit::TestCase
+ sub_test_case(".try_convert") do
+ def test_string
+ expression = "expression"
+ criteria = ArrowFlight::Criteria.try_convert(expression)
+ assert_equal(expression,
+ criteria.expression.to_s)
end
end
end
diff --git a/ruby/red-arrow-flight/test/test-record-batch-reader.rb
b/ruby/red-arrow-flight/test/test-record-batch-reader.rb
index 7f8c64c003..6ecf2ad921 100644
--- a/ruby/red-arrow-flight/test/test-record-batch-reader.rb
+++ b/ruby/red-arrow-flight/test/test-record-batch-reader.rb
@@ -19,6 +19,7 @@ class TestRecordBatchReader < Test::Unit::TestCase
def setup
@server = nil
omit("Unstable on Windows") if Gem.win_platform?
+ omit("Unstable on macOS") if /darwin/.match?(RUBY_PLATFORM)
@server = Helper::Server.new
@server.listen("grpc://127.0.0.1:0")
@location = "grpc://127.0.0.1:#{@server.port}"
diff --git a/ruby/red-arrow-format/Rakefile b/ruby/red-arrow-format/Rakefile
index 3671f35d6e..5187eae19b 100644
--- a/ruby/red-arrow-format/Rakefile
+++ b/ruby/red-arrow-format/Rakefile
@@ -35,7 +35,9 @@ task default: :test
desc "Run tests"
task :test do
cd(base_dir.to_s) do
- ruby("test/run.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run.rb", *args)
end
end
diff --git a/ruby/red-arrow/Rakefile b/ruby/red-arrow/Rakefile
index dd2c310b6d..0142a12c40 100644
--- a/ruby/red-arrow/Rakefile
+++ b/ruby/red-arrow/Rakefile
@@ -76,7 +76,9 @@ end
desc "Run tests"
task :test do
cd(base_dir) do
- ruby("test/run-test.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run-test.rb", *args)
end
end
diff --git a/ruby/red-gandiva/Rakefile b/ruby/red-gandiva/Rakefile
index 579b946d4e..6c2057187f 100644
--- a/ruby/red-gandiva/Rakefile
+++ b/ruby/red-gandiva/Rakefile
@@ -34,7 +34,9 @@ task :test do
cd("dependency-check") do
ruby("-S", "rake")
end
- ruby("test/run-test.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run-test.rb", *args)
end
end
diff --git a/ruby/red-parquet/Rakefile b/ruby/red-parquet/Rakefile
index 579b946d4e..6c2057187f 100644
--- a/ruby/red-parquet/Rakefile
+++ b/ruby/red-parquet/Rakefile
@@ -34,7 +34,9 @@ task :test do
cd("dependency-check") do
ruby("-S", "rake")
end
- ruby("test/run-test.rb")
+ args = []
+ args << "-v" if ENV["RUNNER_DEBUG"]
+ ruby("test/run-test.rb", *args)
end
end
