efl-csharp: fix resource deallocation causing errors everywhere

Summary:
This commit mainly fixes errors caused by deallocating resources in the garbage
collector thread. Using `ecore_main_loop_thread_safe_call_async` to queue
resource deallocation in the main thread seems to solve it.

Also, some `efl_ref` calls are added in places they were missing, mainly
objects that unref in the destructor thus taking ownership if efl_ref is not
called.

Also fix improper resource deallocation in tests that were causing it to crash,
enabling it to call Efl.All.Shutdown again. This allocation and the deallocation
process was moved from the Eo class constructor to static class methods that are
called in the test 'set up' and 'tear down' methods.

Queuing resource deallocation in the main thread make it mandatory that tests
call `Efl.App.AppMain.Iterate()` if they want to check proper resource
deallocation (like TestFunctionPointers.set_callback_inherited_called_from_c).

Extras:
Remove duplicated declaration of 'eflcustomexportsmono' in meson in order to fix
some linking problems.

Remove some unused code around deallocation functions that had to be reworked.

Object allocation is now supplied with the call site information it expects
(file name and line for _efl_add_start).

Depends on D8550

Test Plan: meson test

Reviewers: felipealmeida, lauromoura, cedric, segfaultxavi

Reviewed By: lauromoura

Subscribers: segfaultxavi

Tags: #efl_language_bindings, #do_not_merge

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

src/bin/eolian_mono/eolian/mono/function_pointer.hh

@@ -56,7 +56,7 @@ struct function_pointer {
           return false;
 
       // Wrapper type, with callback matching the Unamanaged one
-      if (!as_generator("internal class " << f_name << "Wrapper\n"
+      if (!as_generator("internal class " << f_name << "Wrapper : IDisposable\n"
                   << "{\n\n"
                   << scope_tab << "private " << f_name  << "Internal _cb;\n"
                   << scope_tab << "private IntPtr _cb_data;\n"
@@ -71,8 +71,31 @@ struct function_pointer {
 
                   << scope_tab << "~" << f_name << "Wrapper()\n"
                   << scope_tab << "{\n"
+                  << scope_tab << scope_tab << "Dispose(false);\n"
+                  << scope_tab << "}\n\n"
+
+                  << scope_tab << "protected virtual void Dispose(bool disposing)\n"
+                  << scope_tab << "{\n"
                   << scope_tab << scope_tab << "if (this._cb_free_cb != null)\n"
-                  << scope_tab << scope_tab << scope_tab << "this._cb_free_cb(this._cb_data);\n"
+                  << scope_tab << scope_tab << "{\n"
+                  << scope_tab << scope_tab << scope_tab << "if (disposing)\n"
+                  << scope_tab << scope_tab << scope_tab << "{\n"
+                  << scope_tab << scope_tab << scope_tab << scope_tab << "this._cb_free_cb(this._cb_data);\n"
+                  << scope_tab << scope_tab << scope_tab << "}\n"
+                  << scope_tab << scope_tab << scope_tab << "else\n"
+                  << scope_tab << scope_tab << scope_tab << "{\n"
+                  << scope_tab << scope_tab << scope_tab << scope_tab << "Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(this._cb_free_cb, this._cb_data);\n"
+                  << scope_tab << scope_tab << scope_tab << "}\n"
+                  << scope_tab << scope_tab << scope_tab << "this._cb_free_cb = null;\n"
+                  << scope_tab << scope_tab << scope_tab << "this._cb_data = IntPtr.Zero;\n"
+                  << scope_tab << scope_tab << scope_tab << "this._cb = null;\n"
+                  << scope_tab << scope_tab << "}\n"
+                  << scope_tab << "}\n\n"
+
+                  << scope_tab << "public void Dispose()\n"
+                  << scope_tab << "{\n"
+                  << scope_tab << scope_tab << "Dispose(true);\n"
+                  << scope_tab << scope_tab << "GC.SuppressFinalize(this);\n"
                   << scope_tab << "}\n\n"
 
                   << scope_tab << "internal " << type << " ManagedCb(" << (parameter % ",") << ")\n"

src/bin/eolian_mono/eolian/mono/klass.hh

@@ -600,7 +600,14 @@ struct klass
              << scope_tab << visibility << "void Dispose(bool disposing)\n"
              << scope_tab << "{\n"
              << scope_tab << scope_tab << "if (handle != System.IntPtr.Zero) {\n"
-             << scope_tab << scope_tab << scope_tab << "Efl.Eo.Globals.efl_unref(handle);\n"
+             << scope_tab << scope_tab << scope_tab << "if (disposing)\n"
+             << scope_tab << scope_tab << scope_tab << "{\n"
+             << scope_tab << scope_tab << scope_tab << scope_tab << "Efl.Eo.Globals.efl_unref(handle);\n"
+             << scope_tab << scope_tab << scope_tab << "}\n"
+             << scope_tab << scope_tab << scope_tab << "else\n"
+             << scope_tab << scope_tab << scope_tab << "{\n"
+             << scope_tab << scope_tab << scope_tab << scope_tab << "Efl.Eo.Globals.efl_mono_thread_safe_efl_unref(handle);\n"
+             << scope_tab << scope_tab << scope_tab << "}\n"
              << scope_tab << scope_tab << scope_tab << "handle = System.IntPtr.Zero;\n"
              << scope_tab << scope_tab << "}\n"
              << scope_tab << "}\n"

src/bindings/mono/efl_mono/meson.build

@@ -8,13 +8,6 @@ bash = find_program('bash')
 map = run_command('map_generate.sh').stdout()
 
 
-efl_mono_lib = library('eflcustomexportsmono',
-    join_paths('..', '..', '..', 'lib', 'efl_mono', 'efl_custom_exports_mono.c'),
-    install : true,
-    install_dir : join_paths(dir_lib, 'efl-mono-'+version_major),
-    dependencies : [eo, eina]
-)
-
 efl_libs = configuration_data()
 efl_libs.set('EFL_MONO_LIBRARY_MAP', map)
 efl_libs.set('CUSTOM_EXPORTS_MONO_DL_MONO', 'eflcustomexportsmono')

src/bindings/mono/eina_mono/eina_accessor.cs

@@ -72,7 +72,14 @@ public class Accessor<T> : IEnumerable<T>, IDisposable
     {
         if (Ownership == Ownership.Managed && Handle != IntPtr.Zero)
         {
-            eina_accessor_free(Handle);
+            if (disposing)
+            {
+                eina_accessor_free(Handle);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_accessor_free, Handle);
+            }
             Handle = IntPtr.Zero;
         }
     }

src/bindings/mono/eina_mono/eina_array.cs

@@ -149,7 +149,14 @@ public class Array<T> : IEnumerable<T>, IDisposable
 
         if (Own)
         {
-            eina_array_free(h);
+            if (disposing)
+            {
+                eina_array_free(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_array_free, h);
+            }
         }
     }
 

src/bindings/mono/eina_mono/eina_binbuf.cs

@@ -103,7 +103,14 @@ public class Binbuf : IDisposable
         Handle = IntPtr.Zero;
         if (Own && h != IntPtr.Zero)
         {
-            eina_binbuf_free(Handle);
+            if (disposing)
+            {
+                eina_binbuf_free(Handle);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_binbuf_free, Handle);
+            }
         }
     }
 

src/bindings/mono/eina_mono/eina_container_common.cs

@@ -229,7 +229,7 @@ public class EflObjectElementTraits<T> : IBaseElementTraits<T>
     {
         if (nat != IntPtr.Zero)
         {
-            Efl.Eo.Globals.efl_unref(nat);
+	    Efl.Eo.Globals.efl_mono_thread_safe_efl_unref(nat);
         }
     }
 

src/bindings/mono/eina_mono/eina_hash.cs

@@ -187,7 +187,14 @@ public class Hash<TKey, TValue> : IEnumerable<KeyValuePair<TKey, TValue>>, IDi
 
         if (Own)
         {
-            eina_hash_free(h);
+            if (disposing)
+            {
+                eina_hash_free(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_hash_free, h);
+            }
         }
     }
 

src/bindings/mono/eina_mono/eina_inarray.cs

@@ -141,7 +141,14 @@ public class Inarray<T> : IEnumerable<T>, IDisposable
 
         if (Own)
         {
-            eina_inarray_free(h);
+            if (disposing)
+            {
+                eina_inarray_free(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_inarray_free, h);
+            }
         }
     }
 

src/bindings/mono/eina_mono/eina_iterator.cs

@@ -73,7 +73,14 @@ public class Iterator<T> : IEnumerable<T>, IDisposable
 
         if (Own)
         {
-            eina_iterator_free(h);
+            if (disposing)
+            {
+                eina_iterator_free(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_iterator_free, h);
+            }
         }
     }
 

src/bindings/mono/eina_mono/eina_list.cs

@@ -45,6 +45,8 @@ public static class ListNativeFunctions
         eina_list_move_list(ref IntPtr to, ref IntPtr from, IntPtr data);
     [DllImport(efl.Libs.Eina)] public static extern IntPtr
         eina_list_free(IntPtr list);
+    [DllImport(efl.Libs.CustomExports)] public static extern void
+        efl_mono_thread_safe_eina_list_free(IntPtr list);
     [DllImport(efl.Libs.Eina)] public static extern IntPtr
         eina_list_nth(IntPtr list, uint n);
     [DllImport(efl.Libs.Eina)] public static extern IntPtr
@@ -190,7 +192,14 @@ public class List<T> : IEnumerable<T>, IDisposable
 
         if (Own)
         {
-            eina_list_free(h);
+            if (disposing)
+            {
+                eina_list_free(h);
+            }
+            else
+            {
+                efl_mono_thread_safe_eina_list_free(h);
+            }
         }
     }
 

src/bindings/mono/eina_mono/eina_promises.cs

@@ -28,6 +28,9 @@ static internal class PromiseNativeMethods
     [DllImport(efl.Libs.Eina)]
     internal static extern void eina_promise_reject(IntPtr scheduler, Eina.Error reason);
 
+    [DllImport(efl.Libs.CustomExports)]
+    internal static extern void efl_mono_thread_safe_promise_reject(IntPtr scheduler, Eina.Error reason);
+
     [DllImport(efl.Libs.Eina)]
     internal static extern IntPtr eina_future_new(IntPtr promise);
 
@@ -148,7 +151,14 @@ public class Promise : IDisposable
     {
         if (Handle != IntPtr.Zero)
         {
-            eina_promise_reject(Handle, Eina.Error.ECANCELED);
+            if (disposing)
+            {
+                eina_promise_reject(Handle, Eina.Error.ECANCELED);
+            }
+            else
+            {
+                efl_mono_thread_safe_promise_reject(Handle, Eina.Error.ECANCELED);
+            }
             Handle = IntPtr.Zero;
         }
     }

src/bindings/mono/eina_mono/eina_strbuf.cs

@@ -93,7 +93,16 @@ public class Strbuf : IDisposable
 
         if (!Disposed && (Handle != IntPtr.Zero))
         {
-            eina_strbuf_free(Handle);
+            if (disposing)
+            {
+                eina_strbuf_free(Handle);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_strbuf_free, Handle);
+            }
+
+            Handle = IntPtr.Zero;
         }
 
         Disposed = true;

src/bindings/mono/eina_mono/eina_value.cs

@@ -1405,7 +1405,14 @@ public class Value : IDisposable, IComparable<Value>, IEquatable<Value>
         if (!Disposed && (Handle != IntPtr.Zero))
         {
             // No need to call flush as eina_value_free already calls it for us.
-            Free(this.Handle);
+            if (disposing)
+            {
+                Free(this.Handle);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eina_value_free, this.Handle);
+            }
         }
 
         Disposed = true;

src/bindings/mono/eldbus_mono/eldbus_connection.cs

@@ -161,7 +161,14 @@ public class Connection : IDisposable
 
         if (Own)
         {
-            eldbus_connection_unref(h);
+            if (disposing)
+            {
+                eldbus_connection_unref(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eldbus_connection_unref, h);
+            }
         }
     }
 

src/bindings/mono/eldbus_mono/eldbus_message.cs

@@ -230,7 +230,14 @@ public class Message : IDisposable
 
         if (Own)
         {
-            eldbus_message_unref(h);
+            if (disposing)
+            {
+                eldbus_message_unref(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eldbus_message_unref, h);
+            }
         }
     }
 

src/bindings/mono/eldbus_mono/eldbus_object.cs

@@ -152,7 +152,14 @@ public class Object : System.IDisposable
 
         if (Own)
         {
-            eldbus_object_unref(h);
+            if (disposing)
+            {
+                eldbus_object_unref(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eldbus_object_unref, h);
+            }
         }
     }
 

src/bindings/mono/eldbus_mono/eldbus_proxy.cs

@@ -111,7 +111,14 @@ public class Proxy : IDisposable
 
         if (Own)
         {
-            eldbus_proxy_unref(h);
+            if (disposing)
+            {
+                eldbus_proxy_unref(h);
+            }
+            else
+            {
+                Efl.Eo.Globals.efl_mono_thread_safe_free_cb_exec(eldbus_proxy_unref, h);
+            }
         }
     }
 

src/bindings/mono/eo_mono/iwrapper.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Runtime.InteropServices;
+using System.Runtime.CompilerServices;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Reflection;
@@ -66,6 +67,11 @@ public class Globals
         efl_ref_count_delegate(IntPtr eo);
     [DllImport(efl.Libs.Eo)] public static extern int
         efl_ref_count(IntPtr eo);
+    [DllImport(efl.Libs.CustomExports)] public static extern void
+        efl_mono_thread_safe_efl_unref(IntPtr eo);
+
+    [DllImport(efl.Libs.CustomExports)] public static extern void
+        efl_mono_thread_safe_free_cb_exec(EinaFreeCb free_cb, IntPtr cb_data);
 
     [DllImport(efl.Libs.Eo)] public static extern IntPtr
         efl_class_name_get(IntPtr eo);
@@ -415,7 +421,9 @@ public class Globals
         }
     }
 
-    public static IntPtr instantiate_start(IntPtr klass, Efl.Object parent)
+    public static IntPtr instantiate_start(IntPtr klass, Efl.Object parent,
+                                           [CallerFilePath] string file = null,
+                                           [CallerLineNumber] int line = 0)
     {
         Eina.Log.Debug($"Instantiating from klass 0x{klass.ToInt64():x}");
         System.IntPtr parent_ptr = System.IntPtr.Zero;
@@ -424,7 +432,7 @@ public class Globals
             parent_ptr = parent.NativeHandle;
         }
 
-        System.IntPtr eo = Efl.Eo.Globals._efl_add_internal_start("file", 0, klass, parent_ptr, 1, 0);
+        System.IntPtr eo = Efl.Eo.Globals._efl_add_internal_start(file, line, klass, parent_ptr, 1, 0);
         if (eo == System.IntPtr.Zero)
         {
             throw new Exception("Instantiation failed");

src/bindings/mono/meson.build

@@ -94,7 +94,8 @@ blacklisted_files = [
 efl_mono_lib = library('eflcustomexportsmono',
     join_paths('..', '..', 'lib', 'efl_mono', 'efl_custom_exports_mono.c'),
     install : true,
-    dependencies : [eo, eina]
+    install_dir : join_paths(dir_lib, 'efl-mono-'+version_major),
+    dependencies : [eo, eina, ecore]
 )
 
 beta_option = []

src/lib/efl_mono/efl_custom_exports_mono.c

@@ -1,5 +1,6 @@
 #include "Eo.h"
 #include "Eina.h"
+#include "Ecore.h"
 
 #include <stdlib.h>
 #include <string.h>
@@ -22,6 +23,52 @@
 # endif
 #endif /* ! _WIN32 */
 
+static void _efl_mono_unref_cb(void *obj)
+{
+   efl_unref(obj);
+}
+
+EAPI void efl_mono_thread_safe_efl_unref(Eo* obj)
+{
+   ecore_main_loop_thread_safe_call_async(_efl_mono_unref_cb, obj);
+}
+
+EAPI void efl_mono_thread_safe_free_cb_exec(Eina_Free_Cb free_cb, void* cb_data)
+{
+   ecore_main_loop_thread_safe_call_async(free_cb, cb_data);
+}
+
+static void _efl_mono_list_free_cb(void *l)
+{
+   eina_list_free(l);
+}
+
+EAPI void efl_mono_thread_safe_eina_list_free(Eina_List* list)
+{
+   ecore_main_loop_thread_safe_call_async(_efl_mono_list_free_cb, list);
+}
+
+typedef struct _Efl_Mono_Promise_Reject_Data
+{
+   Eina_Promise *promise;
+   Eina_Error err;
+} Efl_Mono_Promise_Reject_Data;
+
+static void _efl_mono_promise_reject_cb(void *data)
+{
+   Efl_Mono_Promise_Reject_Data *d = data;
+   eina_promise_reject(d->promise, d->err);
+   free(d);
+}
+
+EAPI void efl_mono_thread_safe_promise_reject(Eina_Promise *p, Eina_Error err)
+{
+   Efl_Mono_Promise_Reject_Data *d = malloc(sizeof(Efl_Mono_Promise_Reject_Data));
+   d->promise = p;
+   d->err = err;
+   ecore_main_loop_thread_safe_call_async(_efl_mono_promise_reject_cb, d);
+}
+
 EAPI void *efl_mono_native_alloc(unsigned int size)
 {
    return malloc(size);
@@ -81,7 +128,7 @@ EAPI Eina_Free_Cb efl_mono_native_free_addr_get()
 
 EAPI Eina_Free_Cb efl_mono_native_efl_unref_addr_get()
 {
-    return (Eina_Free_Cb)efl_unref;
+    return (Eina_Free_Cb)efl_mono_thread_safe_efl_unref;
 }
 
 // Iterator Wrapper //

src/tests/efl_mono/Eina.cs

@@ -454,6 +454,16 @@ class TestEinaSlice
 
 class TestEinaArray
 {
+    public static void SetUp()
+    {
+        Dummy.TestObject.CreateCmpArrayObjects();
+    }
+
+    public static void TearDown()
+    {
+        Dummy.TestObject.DestroyCmpArrayObjects();
+    }
+
     public static void eina_array_default()
     {
         var a = new Eina.Array<int>();
@@ -1273,6 +1283,16 @@ class TestEinaInarray
 
 class TestEinaList
 {
+    public static void SetUp()
+    {
+        Dummy.TestObject.CreateCmpArrayObjects();
+    }
+
+    public static void TearDown()
+    {
+        Dummy.TestObject.DestroyCmpArrayObjects();
+    }
+
     public static void data_set_int()
     {
         var lst = new Eina.List<int>();
@@ -2547,6 +2567,16 @@ class TestEinaHash
 
 class TestEinaIterator
 {
+    public static void SetUp()
+    {
+        Dummy.TestObject.CreateCmpArrayObjects();
+    }
+
+    public static void TearDown()
+    {
+        Dummy.TestObject.DestroyCmpArrayObjects();
+    }
+
     // Array //
 
     public static void eina_array_int_empty_iterator()

src/tests/efl_mono/FunctionPointers.cs

@@ -173,6 +173,7 @@ class TestFunctionPointers
 
         GC.Collect();
         GC.WaitForPendingFinalizers();
+        Efl.App.AppMain.Iterate();
 
         Test.Assert(obj.set_called, "set_callback override must have been called");
         Test.Assert(!obj.invoke_called, "invoke_callback must not have been called");

src/tests/efl_mono/Main.cs

@@ -92,6 +92,8 @@ class TestMain
         }
         Console.WriteLine("[   END SUITE ] " + ckRunSuite);
 
+        Efl.All.Shutdown();
+
         if (!pass)
           return -1;
 

src/tests/efl_mono/dummy_test_object.eo

@@ -1356,6 +1356,12 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface {
          }
       }
 
+      create_cmp_array_objects @class {
+      }
+
+      destroy_cmp_array_objects @class {
+      }
+
       /* Futures */
 
       get_future {
@@ -1405,8 +1411,6 @@ class Dummy.Test_Object extends Efl.Object implements Dummy.Test_Iface {
       }
    }
    implements {
-      class.constructor;
-      class.destructor;
       Efl.Object.constructor;
       Efl.Object.provider_find;
       Dummy.Test_Iface.emit_nonconflicted;

src/tests/efl_mono/libefl_mono_native_test.c

@@ -3640,12 +3640,11 @@ Dummy_StructComplex* _dummy_test_object_struct_complex_ptr_return_own(EINA_UNUSE
 }
 
 //                   //
-// Class constructor
+// Class methods
 //                   //
 EOLIAN static void
-_dummy_test_object_class_constructor(Efl_Class *klass)
+_dummy_test_object_create_cmp_array_objects(void)
 {
-   (void)klass;
    modified_seq_obj[0] = base_seq_obj[0] = _new_obj(0x0);
    modified_seq_obj[1] = base_seq_obj[1] = _new_obj(0x2A);
    modified_seq_obj[2] = base_seq_obj[2] = _new_obj(0x42);
@@ -3655,11 +3654,8 @@ _dummy_test_object_class_constructor(Efl_Class *klass)
 }
 
 EOLIAN static void
-_dummy_test_object_class_destructor(Efl_Class *klass)
+_dummy_test_object_destroy_cmp_array_objects(void)
 {
-   (void)klass;
-   for (unsigned i = 0; i < base_seq_obj_size; ++i)
-     efl_unref(base_seq_obj[i]);
    for (unsigned i = 0; i < modified_seq_obj_size; ++i)
      efl_unref(modified_seq_obj[i]);
 }