From 81b94b3e35a3dc49298f2ed1d90575a42fa0cf0f Mon Sep 17 00:00:00 2001 From: Lauro Moura Date: Wed, 18 Dec 2019 10:36:29 -0300 Subject: [PATCH] csharp: Fix passing acessor with ownership 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 | 1 + src/bindings/mono/eina_mono/eina_accessor.cs | 6 ++ src/bindings/mono/eo_mono/iwrapper.cs | 39 ++++++-- src/lib/efl_mono/efl_mono_accessors.c | 93 ++++++++++++++++++++ src/tests/efl_mono/Eo.cs | 36 ++++++-- src/tests/efl_mono/dummy_test_object.c | 17 ++++ src/tests/efl_mono/dummy_test_object.eo | 7 ++ 7 files changed, 185 insertions(+), 14 deletions(-) create mode 100644 src/lib/efl_mono/efl_mono_accessors.c diff --git a/src/bindings/mono/efl_mono/meson.build b/src/bindings/mono/efl_mono/meson.build index e93d323747..165b6d55bd 100644 --- a/src/bindings/mono/efl_mono/meson.build +++ b/src/bindings/mono/efl_mono/meson.build @@ -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, diff --git a/src/bindings/mono/eina_mono/eina_accessor.cs b/src/bindings/mono/eina_mono/eina_accessor.cs index 0bb3b788f3..77c9817fe7 100644 --- a/src/bindings/mono/eina_mono/eina_accessor.cs +++ b/src/bindings/mono/eina_mono/eina_accessor.cs @@ -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); } /// Accessors provide an uniform way of accessing Eina containers, diff --git a/src/bindings/mono/eo_mono/iwrapper.cs b/src/bindings/mono/eo_mono/iwrapper.cs index 8ba8c964bd..b4f1af9fcd 100644 --- a/src/bindings/mono/eo_mono/iwrapper.cs +++ b/src/bindings/mono/eo_mono/iwrapper.cs @@ -790,21 +790,44 @@ public static class Globals return wrappedAccessor.Handle; } - var list = new List(); + // 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(data)); + intPtrs[i] = Eina.TraitFunctions.ManagedToNativeAlloc(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 IteratorToIEnumerable(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 index 0000000000..f798b14b11 --- /dev/null +++ b/src/lib/efl_mono/efl_mono_accessors.c @@ -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 diff --git a/src/tests/efl_mono/Eo.cs b/src/tests/efl_mono/Eo.cs index 586a18cba3..7c580480ad 100644 --- a/src/tests/efl_mono/Eo.cs +++ b/src/tests/efl_mono/Eo.cs @@ -257,18 +257,21 @@ class TestVariables class TestEoAccessors { - private static void do_eo_accessors(IEnumerable accessor) + private static void do_eo_accessors(IEnumerable accessor, bool shouldMove=false) { var obj = new Dummy.TestObject(); - IEnumerable acc = obj.CloneAccessor(accessor); + IEnumerable source = shouldMove ? accessor.ToList() : accessor; - var zipped = acc.Zip(accessor, (first, second) => new Tuple(first, second)); + IEnumerable acc = shouldMove ? obj.CloneAccessorOwn(accessor) : obj.CloneAccessor(accessor); + + var zipped = acc.Zip(source, (first, second) => new Tuple(first, second)); foreach (Tuple 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 lst = new Eina.List(); + lst.Append(4); + lst.Append(3); + lst.Append(2); + lst.Append(1); + Eina.Accessor 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 lst = new List(); lst.Add(-1); lst.Add(1); @@ -298,6 +311,17 @@ class TestEoAccessors do_eo_accessors(lst); } + + public static void managed_eo_accessors_own() + { + List lst = new List(); + lst.Add(-1); + lst.Add(1); + lst.Add(4); + lst.Add(42); + + do_eo_accessors(lst, shouldMove : true); + } } class TestEoFinalize diff --git a/src/tests/efl_mono/dummy_test_object.c b/src/tests/efl_mono/dummy_test_object.c index fb87a8c1bd..d9ab519d0a 100644 --- a/src/tests/efl_mono/dummy_test_object.c +++ b/src/tests/efl_mono/dummy_test_object.c @@ -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); diff --git a/src/tests/efl_mono/dummy_test_object.eo b/src/tests/efl_mono/dummy_test_object.eo index 1852160623..cf2ae7ce03 100644 --- a/src/tests/efl_mono/dummy_test_object.eo +++ b/src/tests/efl_mono/dummy_test_object.eo @@ -1621,6 +1621,13 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface { return: accessor @move; } + clone_accessor_own { + params { + @in acc: accessor @move; + } + return: accessor @move; + } + @property setter_only { set {} values {