Repository: apache/arrow
File: c_glib/arrow-glib/compute.cpp (C++)

Code before:
    garrow_execute_context_get_instance_private(GARROW_EXECUTE_CONTEXT(object)))

enum {
  PROP_EXECUTOR = 1,
};

static void
garrow_execute_context_finalize(GObject *object)
{
  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);
  priv->context.reset();
  G_OBJECT_CLASS(garrow_execute_context_parent_class)->finalize(object);
}

static void
garrow_execute_context_set_property(GObject *object,
                                    guint prop_id,
                                    const GValue *value,
                                    GParamSpec *pspec)
{
  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);

  switch (prop_id) {
  case PROP_EXECUTOR:
    {
      auto executor = GARROW_EXECUTOR(g_value_get_object(value));
      auto arrow_executor = garrow_executor_get_raw(executor);
      priv->context =
        std::make_shared<arrow::compute::ExecContext>(arrow::default_memory_pool(),
                                                      arrow_executor);
      break;
    }
  default:
    G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
    break;
  }
}

static void
garrow_execute_context_init(GArrowExecuteContext *object)
{
  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);
  priv->context = nullptr;
}

static void
garrow_execute_context_class_init(GArrowExecuteContextClass *klass)
{
  auto gobject_class = G_OBJECT_CLASS(klass);

  gobject_class->finalize = garrow_execute_context_finalize;

Review comment (bug):
Could you keep a reference to the given executor to prevent from GC?

```diff
diff --git a/c_glib/arrow-glib/compute.cpp b/c_glib/arrow-glib/compute.cpp
index 20f36c72e1..04002e30a4 100644
--- a/c_glib/arrow-glib/compute.cpp
+++ b/c_glib/arrow-glib/compute.cpp
@@ -274,10 +274,11 @@ garrow_compute_initialize(GError **error)
   return garrow::check(error, status, "[compute][initialize]");
 }
 
-typedef struct GArrowExecuteContextPrivate_
+struct GArrowExecuteContextPrivate
 {
   std::shared_ptr<arrow::compute::ExecContext> context;
-} GArrowExecuteContextPrivate;
+  GArrowExecutor *executor;
+};
 
 G_DEFINE_TYPE_WITH_PRIVATE(GArrowExecuteContext, garrow_execute_context, G_TYPE_OBJECT)
 
@@ -289,11 +290,24 @@ enum {
   PROP_EXECUTOR = 1,
 };
 
+static void
+garrow_execute_context_dispose(GObject *object)
+{
+  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);
+
+  if (priv->executor) {
+    g_object_unref(priv->executor);
+    priv->executor = nullptr;
+  }
+
+  G_OBJECT_CLASS(garrow_execute_context_parent_class)->dispose(object);
+}
+
 static void
 garrow_execute_context_finalize(GObject *object)
 {
   auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);
-  priv->context.reset();
+  priv->context.~shared_ptr();
   G_OBJECT_CLASS(garrow_execute_context_parent_class)->finalize(object);
 }
 
@@ -308,8 +322,8 @@ garrow_execute_context_set_property(GObject *object,
   switch (prop_id) {
   case PROP_EXECUTOR:
     {
-      auto executor = GARROW_EXECUTOR(g_value_get_object(value));
-      auto arrow_executor = garrow_executor_get_raw(executor);
+      priv->executor = GARROW_EXECUTOR(g_value_dup_object(value));
+      auto arrow_executor = garrow_executor_get_raw(priv->executor);
       priv->context =
         std::make_shared<arrow::compute::ExecContext>(arrow::default_memory_pool(),
                                                       arrow_executor);
@@ -321,11 +335,29 @@ garrow_execute_context_set_property(GObject *object,
   }
 }
 
+static void
+garrow_execute_context_get_property(GObject *object,
+                                    guint prop_id,
+                                    GValue *value,
+                                    GParamSpec *pspec)
+{
+  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);
+
+  switch (prop_id) {
+  case PROP_EXECUTOR:
+    g_value_set_object(value, priv->executor);
+    break;
+  default:
+    G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
+    break;
+  }
+}
+
 static void
 garrow_execute_context_init(GArrowExecuteContext *object)
 {
   auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);
-  priv->context = nullptr;
+  new (&priv->context) std::shared_ptr<arrow::compute::ExecContext>;
 }
 
 static void
@@ -333,16 +365,25 @@ garrow_execute_context_class_init(GArrowExecuteContextClass *klass)
 {
   auto gobject_class = G_OBJECT_CLASS(klass);
 
+  gobject_class->dispose = garrow_execute_context_dispose;
   gobject_class->finalize = garrow_execute_context_finalize;
   gobject_class->set_property = garrow_execute_context_set_property;
+  gobject_class->get_property = garrow_execute_context_get_property;
 
   GParamSpec *spec;
+  /**
+   * GArrowExecuteContext:executor:
+   *
+   * The executor for execution.
+   *
+   * Since: 23.0.0
+   */
   spec = g_param_spec_object(
     "executor",
     "Executor",
-    "The GArrowExecutor for execution",
+    "The executor for execution",
     GARROW_TYPE_EXECUTOR,
-    static_cast<GParamFlags>(G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY));
+    static_cast<GParamFlags>(G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
   g_object_class_install_property(gobject_class, PROP_EXECUTOR, spec);
 }
 
diff --git a/c_glib/test/test-execute-context.rb b/c_glib/test/test-execute-context.rb
new file mode 100644
index 0000000000..fc6873d0a3
--- /dev/null
+++ b/c_glib/test/test-execute-context.rb
@@ -0,0 +1,30 @@
+# 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 TestExecuteContext < Test::Unit::TestCase
+  def setup
+    @context = Arrow::ExecuteContext.new
+  end
+
+  def test_executor
+    assert_nil(@context.executor)
+
+    executor = Arrow::ThreadPool.new(1)
+    context = Arrow::ExecuteContext.new(executor)
+    assert_equal(executor, context.executor)
+  end
+end
```

* We need to use `dispose` not `finalize` for GObject based children for cycled reference. See also: https://docs.gtk.org/gobject/tutorial.html#object-destruction


Code after:
  static_cast<GArrowExecuteContextPrivate *>(                                            \
    garrow_execute_context_get_instance_private(GARROW_EXECUTE_CONTEXT(object)))

enum {
  PROP_EXECUTOR = 1,
};

static void
garrow_execute_context_dispose(GObject *object)
{
  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);

  if (priv->executor) {
    g_object_unref(priv->executor);
    priv->executor = nullptr;
  }

  G_OBJECT_CLASS(garrow_execute_context_parent_class)->dispose(object);
}

static void
garrow_execute_context_finalize(GObject *object)
{
  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);
  priv->context.~shared_ptr();
  G_OBJECT_CLASS(garrow_execute_context_parent_class)->finalize(object);
}

static void
garrow_execute_context_set_property(GObject *object,
                                    guint prop_id,
                                    const GValue *value,
                                    GParamSpec *pspec)
{
  auto priv = GARROW_EXECUTE_CONTEXT_GET_PRIVATE(object);

  switch (prop_id) {
  case PROP_EXECUTOR:
    {
      priv->executor = GARROW_EXECUTOR(g_value_dup_object(value));
      auto arrow_executor = garrow_executor_get_raw(priv->executor);
      priv->context =
        std::make_shared<arrow::compute::ExecContext>(arrow::default_memory_pool(),
                                                      arrow_executor.get());
      break;
    }
  default:
    G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
    break;
  }
}