GH-50302: [GLib][Ruby][FlightRPC] Fix GC related problems (#50401)

kou · web-flow · commit 463682df3b4e · 2026-07-09T09:38:55.000+09:00
### 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 <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
diff --git 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
@@ -22,7 +22,7 @@ def setup
     @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 @@ def test_success
 
     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
@@ -22,7 +22,8 @@ def setup
 
   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
@@ -21,7 +21,8 @@ def setup
   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 @@ def test_ticket
   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 @@ def test_locations
 
   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 @@ def test_true
     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
@@ -22,23 +22,28 @@ def setup
 
   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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/criteria.rb b/ruby/red-arrow-flight/lib/arrow-flight/criteria.rb
@@ -0,0 +1,39 @@
+# 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 ArrowFlight
+  class Criteria
+    class << self
+      def try_convert(value)
+        case value
+        when String, GLib::Bytes
+          new(value)
+        else
+          nil
+        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
@@ -32,6 +32,7 @@ def require_libraries
       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
@@ -28,5 +28,13 @@ def try_convert(value)
         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/test/test-criteria.rb b/ruby/red-arrow-flight/test/test-criteria.rb
@@ -0,0 +1,27 @@
+# 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.
+
+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
@@ -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
@@ -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
@@ -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
diff --git a/ruby/red-parquet/Rakefile b/ruby/red-parquet/Rakefile
