From 840613235de6cabb878aadc163b8a46f7fb4440b Mon Sep 17 00:00:00 2001 From: Lauro Moura Date: Mon, 3 Sep 2018 16:19:21 -0300 Subject: [PATCH] efl-csharp: Use value_new/free for wrapped values Summary: Using malloc/free as it was used before would cause double frees and other issues when mixing with eina_values created from the value mempool inside Eina. Fixes T7359 Reviewers: felipealmeida, vitor.sousa, segfaultxavi Reviewed By: vitor.sousa Subscribers: cedric, #reviewers, #committers Tags: #efl Maniphest Tasks: T7359 Differential Revision: https://phab.enlightenment.org/D6958 --- src/bindings/mono/eina_mono/eina_value.cs | 54 ++++++++++++++------ src/tests/efl_mono/libefl_mono_native_test.c | 6 +-- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/bindings/mono/eina_mono/eina_value.cs b/src/bindings/mono/eina_mono/eina_value.cs index 578c660b91..e0e2f5f76d 100644 --- a/src/bindings/mono/eina_mono/eina_value.cs +++ b/src/bindings/mono/eina_mono/eina_value.cs @@ -38,6 +38,12 @@ struct Value_List [SuppressUnmanagedCodeSecurityAttribute] static internal class UnsafeNativeMethods { + [DllImport(efl.Libs.Eina)] + internal static extern IntPtr eina_value_new(IntPtr type); + + [DllImport(efl.Libs.Eina)] + internal static extern void eina_value_free(IntPtr type); + [DllImport(efl.Libs.Eina)] [return: MarshalAsAttribute(UnmanagedType.U1)] internal static extern bool eina_value_convert(IntPtr handle, IntPtr convert); @@ -392,6 +398,11 @@ public struct Value_Native { public IntPtr Type; public IntPtr Value; // Atually an Eina_Value_Union, but it is padded to 8 bytes. + + public string ToString() + { + return $"Value_Native"; + } } @@ -705,9 +716,19 @@ public class Value : IDisposable, IComparable, IEquatable } } + private static IntPtr Alloc() + { + return eina_value_new(type_int32()); + } + + private static void Free(IntPtr ptr) + { + eina_value_free(ptr); + } + // Constructor to be used by the "FromContainerDesc" methods. private Value() { - this.Handle = MemoryNative.Alloc(eina_value_sizeof()); + this.Handle = Alloc(); this.Ownership = Ownership.Managed; } @@ -722,7 +743,7 @@ public class Value : IDisposable, IComparable, IEquatable if (type.IsContainer()) throw new ArgumentException("To use container types you must provide a subtype"); - this.Handle = MemoryNative.Alloc(eina_value_sizeof()); + this.Handle = Alloc(); if (this.Handle == IntPtr.Zero) throw new OutOfMemoryException("Failed to allocate memory for eina.Value"); @@ -739,7 +760,7 @@ public class Value : IDisposable, IComparable, IEquatable if (!containerType.IsContainer()) throw new ArgumentException("First type must be a container type."); - this.Handle = MemoryNative.Alloc(eina_value_sizeof()); + this.Handle = Alloc(); this.Ownership = Ownership.Managed; Setup(containerType, subtype, step); @@ -748,27 +769,30 @@ public class Value : IDisposable, IComparable, IEquatable /// Constructor to build value from Values_Natives passed by value from C. public Value(Value_Native value) { - IntPtr tmp = MemoryNative.Alloc(Marshal.SizeOf(typeof(Value_Native))); + IntPtr tmp = IntPtr.Zero; try { - Marshal.StructureToPtr(value, tmp, false); // Can't get the address of a struct directly. + this.Handle = Alloc(); if (value.Type == IntPtr.Zero) // Got an EINA_VALUE_EMPTY by value. - { - this.Handle = tmp; - tmp = IntPtr.Zero; - } + MemoryNative.Memset(this.Handle, 0, Marshal.SizeOf(typeof(Value_Native))); else { - this.Handle = MemoryNative.Alloc(Marshal.SizeOf(typeof(Value_Native))); + // We allocate this intermediate Value_Native using malloc to allow freeing with + // free(), avoiding a call to eina_value_flush that would wipe the underlying value contents + // for pointer types like string. + tmp = MemoryNative.Alloc(Marshal.SizeOf(typeof(Value_Native))); + Marshal.StructureToPtr(value, tmp, false); // Can't get the address of a struct directly. + this.Handle = Alloc(); // Copy is used to deep copy the pointed payload (e.g. strings) inside this struct, so we can own this value. if (!eina_value_copy(tmp, this.Handle)) throw new System.InvalidOperationException("Failed to copy value to managed memory."); } } catch { - MemoryNative.Free(this.Handle); + Free(this.Handle); throw; } finally { - MemoryNative.Free(tmp); + if (tmp != IntPtr.Zero) + MemoryNative.Free(tmp); } this.Ownership = Ownership.Managed; @@ -834,10 +858,8 @@ public class Value : IDisposable, IComparable, IEquatable } if (!Disposed && (Handle != IntPtr.Zero)) { - if (!Flushed && !Empty) - eina_value_flush_wrapper(this.Handle); - - MemoryNative.Free(this.Handle); + // No need to call flush as eina_value_free already calls it for us. + Free(this.Handle); } Disposed = true; } diff --git a/src/tests/efl_mono/libefl_mono_native_test.c b/src/tests/efl_mono/libefl_mono_native_test.c index caf7fd98a8..6a746abb92 100644 --- a/src/tests/efl_mono/libefl_mono_native_test.c +++ b/src/tests/efl_mono/libefl_mono_native_test.c @@ -3642,7 +3642,7 @@ void _test_testing_set_value_ptr(EINA_UNUSED Eo *obj, Test_Testing_Data *pd, Ein free(pd->stored_value); } - pd->stored_value = malloc(sizeof(Eina_Value)); + pd->stored_value = eina_value_new(EINA_VALUE_TYPE_INT); eina_value_copy(value, pd->stored_value); } @@ -3662,7 +3662,7 @@ void _test_testing_set_value(EINA_UNUSED Eo *obj, Test_Testing_Data *pd, Eina_Va if (pd->stored_value) { eina_value_free(pd->stored_value); } else { - pd->stored_value = malloc(sizeof(Eina_Value)); + pd->stored_value = eina_value_new(EINA_VALUE_TYPE_INT); } eina_value_copy(&value, pd->stored_value); } @@ -3688,7 +3688,7 @@ void _test_testing_clear_value(EINA_UNUSED Eo *obj, Test_Testing_Data *pd) { if (pd->stored_value) { eina_value_free(pd->stored_value); - free(pd->stored_value); + pd->stored_value = NULL; } }