csharp: Fix passing acessor with ownership
authorLauro Moura <lauromoura@expertisesolutions.com.br>
Wed, 18 Dec 2019 13:36:29 +0000 (10:36 -0300)
committerJongmin Lee <jm105.lee@samsung.com>
Wed, 18 Dec 2019 20:56:57 +0000 (05:56 +0900)
Summary:
When passing an owned acessor from a converted collection we need a way
to unpin the passed data when the accessor is freed.

This commits adds a thin wrapper around the CArray accessor that unpins
the data when freed.

Depends on D10900

Reviewers: YOhoho, felipealmeida

Reviewed By: YOhoho

Subscribers: cedric, #reviewers, #committers, jptiz, brunobelo

Tags: #efl

Maniphest Tasks: T8486

Differential Revision: https://phab.enlightenment.org/D10901

src/bindings/mono/efl_mono/meson.build
src/bindings/mono/eina_mono/eina_accessor.cs
src/bindings/mono/eo_mono/iwrapper.cs
src/lib/efl_mono/efl_mono_accessors.c [new file with mode: 0644]
src/tests/efl_mono/Eo.cs
src/tests/efl_mono/dummy_test_object.c
src/tests/efl_mono/dummy_test_object.eo

index e93d323..165b6d5 100644 (file)
@@ -83,6 +83,7 @@ endforeach
 efl_mono_lib = library('eflcustomexportsmono',
     [
       join_paths('..', '..', '..', 'lib', 'efl_mono', 'efl_custom_exports_mono.c'),
+      join_paths('..', '..', '..', 'lib', 'efl_mono', 'efl_mono_accessors.c'),
       join_paths('..', '..', '..', 'lib', 'efl_mono', 'efl_mono_model_internal.c'),
     ],
     pub_eo_file_target,
index 0bb3b78..77c9817 100644 (file)
@@ -35,6 +35,12 @@ internal class AccessorNativeFunctions
         eina_accessor_data_get(IntPtr accessor, uint position, out IntPtr data);
     [DllImport(efl.Libs.Eina)] public static extern void
         eina_accessor_free(IntPtr accessor);
+    [DllImport(efl.Libs.CustomExports)] public static extern IntPtr
+        eina_mono_owned_carray_length_accessor_new(IntPtr array,
+                                                   uint step,
+                                                   uint length,
+                                                   Eina.Callbacks.EinaFreeCb freeCb,
+                                                   IntPtr handle);
 }
 
 /// <summary>Accessors provide an uniform way of accessing Eina containers,
index 8ba8c96..b4f1af9 100644 (file)
@@ -790,21 +790,44 @@ public static class Globals
             return wrappedAccessor.Handle;
         }
 
-        var list = new List<IntPtr>();
+        // TODO: Check if we're either an Eina.List or Eina.Collection?
+        // We could just rewrap their native accessors
+        IntPtr[] intPtrs = new IntPtr[enumerable.Count()];
+
+        int i = 0;
         foreach (T data in enumerable)
         {
-            list.Add(Eina.TraitFunctions.ManagedToNativeAlloc<T>(data));
+            intPtrs[i] = Eina.TraitFunctions.ManagedToNativeAlloc<T>(data);
+            i++;
         }
-        IntPtr[] dataArray = list.ToArray();
-        GCHandle pinnedArray = GCHandle.Alloc(dataArray, GCHandleType.Pinned); //FIXME: Need to free.
-        IntPtr ret = Eina.AccessorNativeFunctions.eina_carray_length_accessor_new(pinnedArray.AddrOfPinnedObject(), (uint)(IntPtr.Size), (uint)dataArray.Length);
 
-        if (!isMoved)
+        GCHandle pinnedArray = GCHandle.Alloc(intPtrs, GCHandleType.Pinned);
+        IntPtr nativeAccessor = IntPtr.Zero;
+
+        if (isMoved)
         {
-            // FIXME Need to free ret and unpin pinnedArray in the future.
+            // We need a custom accessor that would unpin the data when freed.
+            nativeAccessor = Eina.AccessorNativeFunctions.eina_mono_owned_carray_length_accessor_new(pinnedArray.AddrOfPinnedObject(),
+                                                                                                     (uint)IntPtr.Size,
+                                                                                                     (uint)intPtrs.Length,
+                                                                                                     free_gchandle,
+                                                                                                     GCHandle.ToIntPtr(pinnedArray));
+        }
+        else
+        {
+            // FIXME: Leaking....
+            nativeAccessor = Eina.AccessorNativeFunctions.eina_carray_length_accessor_new(pinnedArray.AddrOfPinnedObject(),
+                                                                                          (uint)(IntPtr.Size),
+                                                                                          (uint)intPtrs.Length);
         }
 
-        return ret;
+        if (nativeAccessor == IntPtr.Zero)
+        {
+            pinnedArray.Free();
+            throw new InvalidOperationException("Failed to get native accessor for the given container");
+        }
+
+        return nativeAccessor;
     }
 
     internal static IEnumerable<T> IteratorToIEnumerable<T>(IntPtr iterator)
diff --git a/src/lib/efl_mono/efl_mono_accessors.c b/src/lib/efl_mono/efl_mono_accessors.c
new file mode 100644 (file)
index 0000000..f798b14
--- /dev/null
@@ -0,0 +1,93 @@
+/*
+ * Copyright 2019 by its authors. See AUTHORS.
+ *
+ * Licensed 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.
+ */
+
+
+
+#include "Eina.h"
+
+#ifdef EAPI
+# undef EAPI
+#endif
+
+#ifdef _WIN32
+#  define EAPI __declspec(dllexport)
+#else
+# ifdef __GNUC__
+#  if __GNUC__ >= 4
+#   define EAPI __attribute__ ((visibility("default")))
+#  else
+#   define EAPI
+#  endif
+# else
+#  define EAPI
+# endif
+#endif /* ! _WIN32 */
+
+// This just a wrapper around carray acessors for pinned managed data
+// It uses the free callback to unpin the managed data so it can be
+// reclaimed by the GC back in C# world.
+struct _Eina_Mono_Owned_Accessor
+{
+   Eina_Accessor accessor;
+
+   Eina_Accessor *carray_acc;
+   void *free_data;
+   Eina_Free_Cb free_cb;
+};
+
+typedef struct _Eina_Mono_Owned_Accessor Eina_Mono_Owned_Accessor;
+
+static Eina_Bool eina_mono_owned_carray_get_at(Eina_Mono_Owned_Accessor *accessor, unsigned int idx, void **data)
+{
+   return eina_accessor_data_get(accessor->carray_acc, idx, data);
+}
+
+static void** eina_mono_owned_carray_get_container(Eina_Mono_Owned_Accessor *accessor)
+{
+  // Is another accessor a valid container?
+  return (void**)&accessor->carray_acc;
+}
+
+static void eina_mono_owned_carray_free_cb(Eina_Mono_Owned_Accessor* accessor)
+{
+   accessor->free_cb(accessor->free_data);
+
+   free(accessor->carray_acc); // From Eina_CArray_Length_Accessor implementation...
+
+   free(accessor);
+}
+
+EAPI Eina_Accessor *eina_mono_owned_carray_length_accessor_new(void** array, unsigned int step, unsigned int length, Eina_Free_Cb free_cb, void *handle)
+{
+   Eina_Mono_Owned_Accessor *accessor = calloc(1, sizeof(Eina_Mono_Owned_Accessor));
+   if (!accessor) return NULL;
+
+   EINA_MAGIC_SET(&accessor->accessor, EINA_MAGIC_ACCESSOR);
+
+   accessor->carray_acc = eina_carray_length_accessor_new(array, step, length);
+
+   accessor->accessor.version = EINA_ACCESSOR_VERSION;
+   accessor->accessor.get_at = FUNC_ACCESSOR_GET_AT(eina_mono_owned_carray_get_at);
+   accessor->accessor.get_container = FUNC_ACCESSOR_GET_CONTAINER(eina_mono_owned_carray_get_container);
+   accessor->accessor.free = FUNC_ACCESSOR_FREE(eina_mono_owned_carray_free_cb);
+
+   // The managed callback to be called with the pinned data.
+   accessor->free_cb = free_cb;
+   // The managed pinned data to be unpinned.
+   accessor->free_data = handle;
+
+   return &accessor->accessor;
+}
\ No newline at end of file
index 586a18c..7c58048 100644 (file)
@@ -257,18 +257,21 @@ class TestVariables
 
 class TestEoAccessors
 {
-    private static void do_eo_accessors(IEnumerable<int> accessor)
+    private static void do_eo_accessors(IEnumerable<int> accessor, bool shouldMove=false)
     {
         var obj = new Dummy.TestObject();
 
-        IEnumerable<int> acc = obj.CloneAccessor(accessor);
+        IEnumerable<int> source = shouldMove ? accessor.ToList() : accessor;
 
-        var zipped = acc.Zip(accessor, (first, second) => new Tuple<int, int>(first, second));
+        IEnumerable<int> acc = shouldMove ? obj.CloneAccessorOwn(accessor) : obj.CloneAccessor(accessor);
+
+        var zipped = acc.Zip(source, (first, second) => new Tuple<int, int>(first, second));
 
         foreach (Tuple<int, int> pair in zipped)
         {
             Test.AssertEquals(pair.Item1, pair.Item2);
         }
+
         obj.Dispose();
     }
 
@@ -280,16 +283,26 @@ class TestEoAccessors
         lst.Append(2);
         lst.Append(5);
 
-        // FIXME: Replace the first accessor with the list once Eina.List implements Eina.IList
         do_eo_accessors(lst.GetAccessor());
 
         lst.Dispose();
     }
+    public static void eina_eo_accessors_own()
+    {
+        Eina.List<int> lst = new Eina.List<int>();
+        lst.Append(4);
+        lst.Append(3);
+        lst.Append(2);
+        lst.Append(1);
+        Eina.Accessor<int> acc = lst.GetAccessor();
+        do_eo_accessors(acc, shouldMove : true);
+
+        Test.Assert(acc.Own);
+
+    }
 
     public static void managed_eo_accessors()
     {
-        var obj = new Dummy.TestObject();
-
         List<int> lst = new List<int>();
         lst.Add(-1);
         lst.Add(1);
@@ -298,6 +311,17 @@ class TestEoAccessors
 
         do_eo_accessors(lst);
     }
+
+    public static void managed_eo_accessors_own()
+    {
+        List<int> lst = new List<int>();
+        lst.Add(-1);
+        lst.Add(1);
+        lst.Add(4);
+        lst.Add(42);
+
+        do_eo_accessors(lst, shouldMove : true);
+    }
 }
 
 class TestEoFinalize
index fb87a8c..d9ab519 100644 (file)
@@ -4683,6 +4683,23 @@ Eina_Accessor *_dummy_test_object_clone_accessor(Eo *obj EINA_UNUSED, Dummy_Test
    return eina_list_accessor_new(pd->list_for_accessor);
 }
 
+Eina_Accessor *_dummy_test_object_clone_accessor_own(Eo *obj EINA_UNUSED, Dummy_Test_Object_Data *pd, Eina_Accessor *acc)
+{
+   if (pd->list_for_accessor)
+     eina_list_free(pd->list_for_accessor);
+
+   unsigned int i;
+   int *data;
+   EINA_ACCESSOR_FOREACH(acc, i, data)
+     {
+         pd->list_for_accessor = eina_list_append(pd->list_for_accessor, data);
+     }
+
+   eina_accessor_free(acc);
+
+   return eina_list_accessor_new(pd->list_for_accessor);
+}
+
 void _dummy_test_object_dummy_test_iface_emit_nonconflicted(Eo *obj, Dummy_Test_Object_Data *pd EINA_UNUSED)
 {
     efl_event_callback_legacy_call(obj, DUMMY_TEST_IFACE_EVENT_NONCONFLICTED, NULL);
index 1852160..cf2ae7c 100644 (file)
@@ -1621,6 +1621,13 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface {
          return: accessor<int> @move;
       }
 
+      clone_accessor_own {
+         params {
+            @in acc: accessor<int> @move;
+         }
+         return: accessor<int> @move;
+      }
+
       @property setter_only {
          set {}
          values {