From fcf5f1d2e2d9ce877d550dd3352bbd6e0527299e Mon Sep 17 00:00:00 2001 From: Vitor Sousa Date: Fri, 31 May 2019 17:43:11 -0300 Subject: [PATCH] csharp: Refactor wrapper lifetime. Summary: This commit makes use of the `ownership,shared` and `ownership,unique` events from Efl.Object in order to avoid the C# wrapper from being collected while C code holds a reference to the object. For example, creating a list of items in a for loop and attaching events to them would fails without this commit, as the C# GC may collect the wrapper. The basic idea is that we use a `WrapperSupervisor`, which is stored in the Eo data storage, with a GCHandle allocated for the lifetime of the underlying Eo object. This supervisor takes care of holding either a weak C# reference (when in unique mode, allowing the wrapper to be GC'd) or a hard C# reference (when in shared mode, making the wrapper non-collectable while the Eo has extra references). One limitation is that object graphs can leak if a shared object in the graph - an Eo child for example - stores a hard reference to another object in the graph as a C# field. In this example, this causes the parent to always have a hard C# reference (from the child) as the child is non-collectable due to the parent holding an Eo reference to it. Depends on D8678 Test Plan: `ninja test` and `make test` Reviewers: lauromoura, felipealmeida, woohyun, segfaultxavi Reviewed By: lauromoura Subscribers: cedric, #reviewers, #committers Tags: #efl Differential Revision: https://phab.enlightenment.org/D9014 --- src/Makefile_Efl_Mono.am | 4 +- src/bin/eolian_mono/eolian/mono/events.hh | 3 +- .../eolian/mono/function_definition.hh | 6 +- src/bin/eolian_mono/eolian/mono/klass.hh | 309 ++---------------- .../mono/eina_mono/eina_container_common.cs | 2 +- src/bindings/mono/eo_mono/EoWrapper.cs | 253 ++++++++++++++ .../mono/eo_mono/WrapperSupervisor.cs | 64 ++++ src/bindings/mono/eo_mono/iwrapper.cs | 235 ++++++------- src/bindings/mono/eo_mono/meson.build | 4 +- src/bindings/mono/eo_mono/workaround.cs | 9 +- src/lib/efl_mono/efl_custom_exports_mono.c | 49 ++- src/tests/efl_mono/Eo.cs | 15 + src/tests/efl_mono/Inheritance.cs | 89 +++++ src/tests/efl_mono/TestUtils.cs | 28 +- src/tests/efl_mono/dummy_part_holder.c | 23 ++ src/tests/efl_mono/dummy_part_holder.eo | 1 + 16 files changed, 621 insertions(+), 473 deletions(-) create mode 100644 src/bindings/mono/eo_mono/EoWrapper.cs create mode 100644 src/bindings/mono/eo_mono/WrapperSupervisor.cs diff --git a/src/Makefile_Efl_Mono.am b/src/Makefile_Efl_Mono.am index 8caf083e05..0cf41589ff 100644 --- a/src/Makefile_Efl_Mono.am +++ b/src/Makefile_Efl_Mono.am @@ -6,7 +6,9 @@ efl_eo_mono_files = \ bindings/mono/eo_mono/iwrapper.cs \ bindings/mono/eo_mono/FunctionWrapper.cs \ bindings/mono/eo_mono/NativeModule.cs \ - bindings/mono/eo_mono/workaround.cs + bindings/mono/eo_mono/workaround.cs \ + bindings/mono/eo_mono/EoWrapper.cs \ + bindings/mono/eo_mono/WrapperSupervisor.cs if HAVE_WIN32 diff --git a/src/bin/eolian_mono/eolian/mono/events.hh b/src/bin/eolian_mono/eolian/mono/events.hh index 350dd57ad4..0522a9be7a 100644 --- a/src/bin/eolian_mono/eolian/mono/events.hh +++ b/src/bin/eolian_mono/eolian/mono/events.hh @@ -431,10 +431,9 @@ struct event_definition_generator << scope_tab << scope_tab << "{\n" << scope_tab << scope_tab << scope_tab << "lock (eventLock)\n" << scope_tab << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "var wRef = new WeakReference(this);\n" << scope_tab << scope_tab << scope_tab << scope_tab << "Efl.EventCb callerCb = (IntPtr data, ref Efl.Event.NativeStruct evt) =>\n" << scope_tab << scope_tab << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << scope_tab << scope_tab << "var obj = wRef.Target as Efl.Eo.IWrapper;\n" + << scope_tab << scope_tab << scope_tab << scope_tab << scope_tab << "var obj = Efl.Eo.Globals.WrapperSupervisorPtrToManaged(data).Target;\n" << scope_tab << scope_tab << scope_tab << scope_tab << scope_tab << "if (obj != null)\n" << scope_tab << scope_tab << scope_tab << scope_tab << scope_tab << "{\n" << scope_tab << scope_tab << scope_tab << scope_tab << scope_tab << scope_tab << event_args diff --git a/src/bin/eolian_mono/eolian/mono/function_definition.hh b/src/bin/eolian_mono/eolian/mono/function_definition.hh index d1ca5f573f..8fc7225bf3 100644 --- a/src/bin/eolian_mono/eolian/mono/function_definition.hh +++ b/src/bin/eolian_mono/eolian/mono/function_definition.hh @@ -104,14 +104,14 @@ struct native_function_definition_generator << ")\n" << indent << "{\n" << indent << scope_tab << "Eina.Log.Debug(\"function " << string << " was called\");\n" - << indent << scope_tab << "Efl.Eo.IWrapper wrapper = Efl.Eo.Globals.PrivateDataGet(pd);\n" - << indent << scope_tab << "if (wrapper != null)\n" + << indent << scope_tab << "var ws = Efl.Eo.Globals.GetWrapperSupervisor(obj);\n" + << indent << scope_tab << "if (ws != null)\n" << indent << scope_tab << "{\n" << eolian_mono::native_function_definition_preamble() << indent << scope_tab << scope_tab << "try\n" << indent << scope_tab << scope_tab << "{\n" << indent << scope_tab << scope_tab << scope_tab << (return_type != "void" ? "_ret_var = " : "") - << (f.is_static ? "" : "((") << klass_cast_name << (f.is_static ? "." : ")wrapper).") << string + << (f.is_static ? "" : "((") << klass_cast_name << (f.is_static ? "." : ")ws.Target).") << string << "(" << (native_argument_invocation % ", ") << ");\n" << indent << scope_tab << scope_tab << "}\n" << indent << scope_tab << scope_tab << "catch (Exception e)\n" diff --git a/src/bin/eolian_mono/eolian/mono/klass.hh b/src/bin/eolian_mono/eolian/mono/klass.hh index 9d3711fb61..6ac4aa1c3c 100644 --- a/src/bin/eolian_mono/eolian/mono/klass.hh +++ b/src/bin/eolian_mono/eolian/mono/klass.hh @@ -31,37 +31,6 @@ namespace eolian_mono { -template -static bool generate_equals_method(OutputIterator sink, Context const &context) -{ - return as_generator( - scope_tab << "/// Verifies if the given object is equal to this one.\n" - << scope_tab << "/// The object to compare to.\n" - << scope_tab << "/// True if both objects point to the same native object.\n" - << scope_tab << "public override bool Equals(object instance)\n" - << scope_tab << "{\n" - << scope_tab << scope_tab << "var other = instance as Efl.Object;\n" - << scope_tab << scope_tab << "if (other == null)\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "return false;\n" - << scope_tab << scope_tab << "}\n" - << scope_tab << scope_tab << "return this.NativeHandle == other.NativeHandle;\n" - << scope_tab << "}\n\n" - << scope_tab << "/// Gets the hash code for this object based on the native pointer it points to.\n" - << scope_tab << "/// The value of the pointer, to be used as the hash code of this object.\n" - << scope_tab << "public override int GetHashCode()\n" - << scope_tab << "{\n" - << scope_tab << scope_tab << "return this.NativeHandle.ToInt32();\n" - << scope_tab << "}\n\n" - << scope_tab << "/// Turns the native pointer into a string representation.\n" - << scope_tab << "/// A string with the type and the native pointer for this object.\n" - << scope_tab << "public override String ToString()\n" - << scope_tab << "{\n" - << scope_tab << scope_tab << "return $\"{this.GetType().Name}@[{this.NativeHandle.ToInt32():x}]\";\n" - << scope_tab << "}\n\n" - ).generate(sink, nullptr, context); -} - /* Get the actual number of functions of a class, checking for blacklisted ones */ template static std::size_t @@ -216,9 +185,9 @@ struct klass if(!as_generator ( documentation - << "sealed public class " << concrete_name << " : " << "\n" - << (klass_full_concrete_or_interface_name % ",") << "\n" - << (inherit_classes.size() > 0 ? ", " : "" ) << interface_name << "\n" + << "sealed public class " << concrete_name << " :\n" + << scope_tab << (root ? "Efl.Eo.EoWrapper" : "") << (klass_full_concrete_or_interface_name % "") << "\n" + << scope_tab << ", " << interface_name << "\n" << scope_tab << *(", " << name_helpers::klass_full_concrete_or_interface_name) << "\n" << "{\n" ).generate(sink, std::make_tuple(cls, inherit_classes, inherit_interfaces), concrete_cxt)) @@ -227,7 +196,6 @@ struct klass if (!generate_fields(sink, cls, concrete_cxt)) return false; - bool root = !helpers::has_regular_ancestor(cls); if (!as_generator ( scope_tab << "[System.Runtime.InteropServices.DllImport(" << context_find_tag(concrete_cxt).actual_library_name(cls.filename) @@ -235,20 +203,13 @@ struct klass << scope_tab << scope_tab << name_helpers::klass_get_name(cls) << "();\n" << scope_tab << "/// Initializes a new instance of the class.\n" << scope_tab << "/// Internal usage: This is used when interacting with C code and should not be used directly.\n" - << scope_tab << "private " << concrete_name << "(System.IntPtr raw)" << (root ? "" : " : base(raw)") << "\n" + << scope_tab << "private " << concrete_name << "(System.IntPtr raw) : base(raw)\n" << scope_tab << "{\n" - << scope_tab << scope_tab << (root ? "handle = raw;\n" : "") - << scope_tab << "}\n" + << scope_tab << "}\n\n" ) .generate(sink, attributes::unused, concrete_cxt)) return false; - if (!generate_dispose_methods(sink, cls, concrete_cxt)) - return false; - - if (!generate_equals_method(sink, concrete_cxt)) - return false; - if (!generate_events(sink, cls, concrete_cxt)) return false; @@ -305,10 +266,9 @@ struct klass << "[" << name_helpers::klass_full_native_inherit_name(cls) << "]\n" << "public " << class_type << " " << name_helpers::klass_concrete_name(cls) << " : " << (klass_full_concrete_or_interface_name % ",") // classes - << (inherit_classes.empty() ? "" : ",") - << " Efl.Eo.IWrapper" << (root ? ", IDisposable" : "") - << (inherit_interfaces.empty() ? "" : ",") - << (klass_full_concrete_or_interface_name % ",") // interfaces + << (root ? "Efl.Eo.EoWrapper" : "") // ... or root + << (inherit_interfaces.empty() ? "" : ", ") + << (klass_full_concrete_or_interface_name % ", ") // interfaces << "\n{\n" ) .generate(sink, std::make_tuple(cls, inherit_classes, inherit_interfaces), inherit_cxt)) @@ -322,12 +282,6 @@ struct klass if (!generate_constructors(sink, cls, inherit_cxt)) return false; - if (!generate_dispose_methods(sink, cls, inherit_cxt)) - return false; - - if (!generate_equals_method(sink, inherit_cxt)) - return false; - if (!generate_events(sink, cls, inherit_cxt)) return false; @@ -479,22 +433,14 @@ struct klass bool generate_fields(OutputIterator sink, attributes::klass_def const& cls, Context const& context) const { std::string visibility = is_inherit_context(context) ? "protected " : "private "; - bool root = !helpers::has_regular_ancestor(cls); - bool is_inherit = is_inherit_context(context); std::string class_getter = name_helpers::klass_get_name(cls) + "()"; std::string native_inherit_full_name = name_helpers::klass_full_native_inherit_name(cls); auto inherit_name = name_helpers::klass_concrete_name(cls); - std::string raw_klass_modifier; - if (!root) - raw_klass_modifier = "override "; - else if (is_inherit) - raw_klass_modifier = "virtual "; - if(!as_generator( scope_tab << "///Pointer to the native class description.\n" - << scope_tab << "public " << raw_klass_modifier << "System.IntPtr NativeClass\n" + << scope_tab << "public override System.IntPtr NativeClass\n" << scope_tab << "{\n" << scope_tab << scope_tab << "get\n" << scope_tab << scope_tab << "{\n" @@ -511,42 +457,12 @@ struct klass ).generate(sink, attributes::unused, context)) return false; - // The remaining fields aren't needed in children classes. - if (!root) - return true; - - if (cls.get_all_events().size() > 0) - if (!as_generator( - scope_tab << "/// Internal usage by derived classes to track native events.\n" - << scope_tab << visibility << "Dictionary<(IntPtr desc, object evtDelegate), (IntPtr evtCallerPtr, Efl.EventCb evtCaller)> eoEvents = new Dictionary<(IntPtr desc, object evtDelegate), (IntPtr evtCallerPtr, Efl.EventCb evtCaller)>();\n" - << scope_tab << "/// Internal usage by derived classes to lock native event handlers.\n" - << scope_tab << visibility << "readonly object eventLock = new object();\n") - .generate(sink, attributes::unused, context)) - return false; - - if (is_inherit) - { - if (!as_generator( - scope_tab << "/// Internal usage to detect whether this instance is from a generated class or not.\n" - << scope_tab << "protected bool inherited;\n" - ).generate(sink, attributes::unused, context)) - return false; - } - - return as_generator( - scope_tab << visibility << " System.IntPtr handle;\n" - << scope_tab << "///Pointer to the native instance.\n" - << scope_tab << "public System.IntPtr NativeHandle\n" - << scope_tab << "{\n" - << scope_tab << scope_tab << "get { return handle; }\n" - << scope_tab << "}\n\n" - ).generate(sink, attributes::unused, context); + return true; } template bool generate_constructors(OutputIterator sink, attributes::klass_def const& cls, Context const& context) const { - bool root = !helpers::has_regular_ancestor(cls); auto inherit_name = name_helpers::klass_concrete_name(cls); if(!as_generator( @@ -571,7 +487,7 @@ struct klass // For constructors with arguments, the parent is also required, as optional parameters can't come before non-optional paramenters. << scope_tab << "public " << inherit_name << "(Efl.Object parent" << ((constructors.size() > 0) ? "" : "= null") << "\n" << scope_tab << scope_tab << scope_tab << *(", " << constructor_param ) << ") : " - << (root ? "this" : "base") << "(" << name_helpers::klass_get_name(cls) << "(), typeof(" << inherit_name << "), parent)\n" + << "base(" << name_helpers::klass_get_name(cls) << "(), typeof(" << inherit_name << "), parent)\n" << scope_tab << "{\n" << (*(scope_tab << scope_tab << constructor_invocation << "\n")) << scope_tab << scope_tab << "FinishInstantiation();\n" @@ -579,9 +495,8 @@ struct klass << scope_tab << "/// Initializes a new instance of the class.\n" << scope_tab << "/// Internal usage: Constructs an instance from a native pointer. This is used when interacting with C code and should not be used directly.\n" << scope_tab << "/// The native pointer to be wrapped.\n" - << scope_tab << "protected " << inherit_name << "(System.IntPtr raw)" << (root ? "" : " : base(raw)") << "\n" + << scope_tab << "protected " << inherit_name << "(System.IntPtr raw) : base(raw)\n" << scope_tab << "{\n" - << scope_tab << scope_tab << (root ? "handle = raw;\n" : "") << scope_tab << "}\n\n" ).generate(sink, std::make_tuple(constructors, constructors, constructors), context)) return false; @@ -603,124 +518,16 @@ struct klass return false; } - // Internal constructors - if (!root) - { - return as_generator( - scope_tab << "/// Initializes a new instance of the class.\n" - << scope_tab << "/// Internal usage: Constructor to forward the wrapper initialization to the root class that interfaces with native code. Should not be used directly.\n" - << scope_tab << "/// The pointer to the base native Eo class.\n" - << scope_tab << "/// The managed type of the public constructor that originated this call.\n" - << scope_tab << "/// The Efl.Object parent of this instance.\n" - << scope_tab << "protected " << inherit_name << "(IntPtr baseKlass, System.Type managedType, Efl.Object parent) : base(baseKlass, managedType, parent)\n" - << scope_tab << "{\n" - << scope_tab << "}\n\n" - ).generate(sink, attributes::unused, context); - - } - - // Detailed constructors go only in root classes. return as_generator( - /// Actual root costructor that creates class and instantiates - scope_tab << "/// Initializes a new instance of the class.\n" - << scope_tab << "/// Internal usage: Constructor to actually call the native library constructors. C# subclasses\n" - << scope_tab << "/// must use the public constructor only.\n" - << scope_tab << "/// The pointer to the base native Eo class.\n" - << scope_tab << "/// The managed type of the public constructor that originated this call.\n" - << scope_tab << "/// The Efl.Object parent of this instance.\n" - << scope_tab << "protected " << inherit_name << "(IntPtr baseKlass, System.Type managedType, Efl.Object parent)\n" - << scope_tab << "{\n" - << scope_tab << scope_tab << "inherited = ((object)this).GetType() != managedType;\n" - << scope_tab << scope_tab << "IntPtr actual_klass = baseKlass;\n" - << scope_tab << scope_tab << "if (inherited)\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "actual_klass = Efl.Eo.ClassRegister.GetInheritKlassOrRegister(baseKlass, ((object)this).GetType());\n" - << scope_tab << scope_tab << "}\n\n" - << scope_tab << scope_tab << "handle = Efl.Eo.Globals.instantiate_start(actual_klass, parent);\n" - << scope_tab << scope_tab << "if (inherited)\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "Efl.Eo.Globals.PrivateDataSet(this);\n" - << scope_tab << scope_tab << "}\n" - << scope_tab << "}\n\n" - - << scope_tab << "/// Finishes instantiating this object.\n" - << scope_tab << "/// Internal usage by generated code.\n" - << scope_tab << "protected void FinishInstantiation()\n" - << scope_tab << "{\n" - << scope_tab << scope_tab << "handle = Efl.Eo.Globals.instantiate_end(handle);\n" - << scope_tab << scope_tab << "Eina.Error.RaiseIfUnhandledException();\n" - << scope_tab << "}\n\n" - - ).generate(sink, attributes::unused, context); - } - - - template - bool generate_dispose_methods(OutputIterator sink, attributes::klass_def const& cls, Context const& context) const - { - if (helpers::has_regular_ancestor(cls)) - return true; - - std::string visibility = is_inherit_context(context) ? "protected virtual " : "private "; - - auto inherit_name = name_helpers::klass_concrete_name(cls); - - std::string events_gchandle; - if (cls.get_all_events().size() > 0) - { - auto events_gchandle_sink = std::back_inserter(events_gchandle); - if (!as_generator(scope_tab << scope_tab << scope_tab << "if (eoEvents.Count != 0)\n" - << scope_tab << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "GCHandle gcHandle = GCHandle.Alloc(eoEvents);\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "gcHandlePtr = GCHandle.ToIntPtr(gcHandle);\n" - << scope_tab << scope_tab << scope_tab << "}\n\n") - .generate(events_gchandle_sink, attributes::unused, context)) - return false; - } - - return as_generator( - - scope_tab << "///Destructor.\n" - << scope_tab << "~" << inherit_name << "()\n" - << scope_tab << "{\n" - << scope_tab << scope_tab << "Dispose(false);\n" - << scope_tab << "}\n\n" - - << scope_tab << "///Releases the underlying native instance.\n" - << 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 << "{\n" - << scope_tab << scope_tab << scope_tab << "IntPtr h = handle;\n" - << scope_tab << scope_tab << scope_tab << "handle = IntPtr.Zero;\n\n" - - << scope_tab << scope_tab << scope_tab << "IntPtr gcHandlePtr = IntPtr.Zero;\n" - << events_gchandle - - << 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_mono_native_dispose(h, gcHandlePtr);\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 << "Monitor.Enter(Efl.All.InitLock);\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "if (Efl.All.MainLoopInitialized)\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << scope_tab << scope_tab << "Efl.Eo.Globals.efl_mono_thread_safe_native_dispose(h, gcHandlePtr);\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "}\n\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "Monitor.Exit(Efl.All.InitLock);\n" - << scope_tab << scope_tab << scope_tab << "}\n" - << scope_tab << scope_tab << "}\n\n" - << scope_tab << "}\n\n" - - << scope_tab << "///Releases the underlying native instance.\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" - ).generate(sink, attributes::unused, context); + scope_tab << "/// Initializes a new instance of the class.\n" + << scope_tab << "/// Internal usage: Constructor to forward the wrapper initialization to the root class that interfaces with native code. Should not be used directly.\n" + << scope_tab << "/// The pointer to the base native Eo class.\n" + << scope_tab << "/// The managed type of the public constructor that originated this call.\n" + << scope_tab << "/// The Efl.Object parent of this instance.\n" + << scope_tab << "protected " << inherit_name << "(IntPtr baseKlass, System.Type managedType, Efl.Object parent) : base(baseKlass, managedType, parent)\n" + << scope_tab << "{\n" + << scope_tab << "}\n\n" + ).generate(sink, attributes::unused, context); } template @@ -730,80 +537,6 @@ struct klass if (!has_events(cls)) return true; - std::string visibility = is_inherit_context(context) ? "protected " : "private "; - - if (!helpers::has_regular_ancestor(cls)) - { - // Callback registration functions - if (!as_generator( - scope_tab << "///Adds a new event handler, registering it to the native event. For internal use only.\n" - << scope_tab << "///The name of the native library definining the event.\n" - << scope_tab << "///The name of the native event.\n" - << scope_tab << "///Delegate to be called by native code on event raising.\n" - << scope_tab << "///Managed delegate that will be called by evtCaller on event raising.\n" - << scope_tab << visibility << "void AddNativeEventHandler(string lib, string key, Efl.EventCb evtCaller, object evtDelegate)\n" - << scope_tab << "{\n" - - << scope_tab << scope_tab << "IntPtr desc = Efl.EventDescription.GetNative(lib, key);\n" - << scope_tab << scope_tab << "if (desc == IntPtr.Zero)\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "Eina.Log.Error($\"Failed to get native event {key}\");\n" - << scope_tab << scope_tab << "}\n\n" - - << scope_tab << scope_tab << "if (eoEvents.ContainsKey((desc, evtDelegate)))\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "Eina.Log.Warning($\"Event proxy for event {key} already registered!\");\n" - << scope_tab << scope_tab << scope_tab << "return;\n" - << scope_tab << scope_tab << "}\n\n" - - << scope_tab << scope_tab << "IntPtr evtCallerPtr = Marshal.GetFunctionPointerForDelegate(evtCaller);\n" - << scope_tab << scope_tab << "if (!Efl.Eo.Globals.efl_event_callback_priority_add(handle, desc, 0, evtCallerPtr, IntPtr.Zero))\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "Eina.Log.Error($\"Failed to add event proxy for event {key}\");\n" - << scope_tab << scope_tab << scope_tab << "return;\n" - << scope_tab << scope_tab << "}\n\n" - - << scope_tab << scope_tab << "eoEvents[(desc, evtDelegate)] = (evtCallerPtr, evtCaller);\n" - << scope_tab << scope_tab << "Eina.Error.RaiseIfUnhandledException();\n" - << scope_tab << "}\n\n" - - << scope_tab << "///Removes the given event handler for the given event. For internal use only.\n" - << scope_tab << "///The name of the native library definining the event.\n" - << scope_tab << "///The name of the native event.\n" - << scope_tab << "///The delegate to be removed.\n" - << scope_tab << visibility << "void RemoveNativeEventHandler(string lib, string key, object evtDelegate)\n" - << scope_tab << "{\n" - - << scope_tab << scope_tab << "IntPtr desc = Efl.EventDescription.GetNative(lib, key);\n" - << scope_tab << scope_tab << "if (desc == IntPtr.Zero)\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "Eina.Log.Error($\"Failed to get native event {key}\");\n" - << scope_tab << scope_tab << scope_tab << "return;\n" - << scope_tab << scope_tab << "}\n\n" - - << scope_tab << scope_tab << "var evtPair = (desc, evtDelegate);\n" - << scope_tab << scope_tab << "if (eoEvents.TryGetValue(evtPair, out var caller))\n" - << scope_tab << scope_tab << "{\n" - - << scope_tab << scope_tab << scope_tab << "if (!Efl.Eo.Globals.efl_event_callback_del(handle, desc, caller.evtCallerPtr, IntPtr.Zero))\n" - << scope_tab << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "Eina.Log.Error($\"Failed to remove event proxy for event {key}\");\n" - << scope_tab << scope_tab << scope_tab << scope_tab << "return;\n" - << scope_tab << scope_tab << scope_tab << "}\n\n" - - << scope_tab << scope_tab << scope_tab << "eoEvents.Remove(evtPair);\n" - << scope_tab << scope_tab << scope_tab << "Eina.Error.RaiseIfUnhandledException();\n" - << scope_tab << scope_tab << "}\n" - << scope_tab << scope_tab << "else\n" - << scope_tab << scope_tab << "{\n" - << scope_tab << scope_tab << scope_tab << "Eina.Log.Error($\"Trying to remove proxy for event {key} when it is nothing registered.\");\n" - << scope_tab << scope_tab << "}\n" - << scope_tab << "}\n\n" - ) - .generate(sink, NULL, context)) - return false; - } - // Self events if (!as_generator(*(event_definition(cls, cls))).generate(sink, cls.events, context)) return false; diff --git a/src/bindings/mono/eina_mono/eina_container_common.cs b/src/bindings/mono/eina_mono/eina_container_common.cs index 630b084207..7bc17d9215 100644 --- a/src/bindings/mono/eina_mono/eina_container_common.cs +++ b/src/bindings/mono/eina_mono/eina_container_common.cs @@ -229,7 +229,7 @@ public class EflObjectElementTraits : IBaseElementTraits { if (nat != IntPtr.Zero) { - Efl.Eo.Globals.efl_mono_thread_safe_efl_unref(nat); + Efl.Eo.Globals.efl_mono_thread_safe_efl_unref(nat); } } diff --git a/src/bindings/mono/eo_mono/EoWrapper.cs b/src/bindings/mono/eo_mono/EoWrapper.cs new file mode 100644 index 0000000000..b6ea619923 --- /dev/null +++ b/src/bindings/mono/eo_mono/EoWrapper.cs @@ -0,0 +1,253 @@ +using System; +using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; +using System.Threading; + +namespace Efl +{ + +namespace Eo +{ + +public abstract class EoWrapper : IWrapper, IDisposable +{ + protected readonly object eventLock = new object(); + protected bool inherited = false; + protected System.IntPtr handle = IntPtr.Zero; + + private static Efl.EventCb ownershipUniqueDelegate = new Efl.EventCb(OwnershipUniqueCallback); + private static Efl.EventCb ownershipSharedDelegate = new Efl.EventCb(OwnershipSharedCallback); + + /// Initializes a new instance of the class. + /// Internal usage: Constructs an instance from a native pointer. This is used when interacting with C code and should not be used directly. + /// The native pointer to be wrapped. + protected EoWrapper(System.IntPtr raw) + { + handle = raw; + AddWrapperSupervisor(); + } + + /// Initializes a new instance of the class. + /// Internal usage: Constructor to actually call the native library constructors. C# subclasses + /// must use the public constructor only. + /// The pointer to the base native Eo class. + /// The managed type of the public constructor that originated this call. + /// The Efl.Object parent of this instance. + /// Name of the file from where the constructor is called. + /// Number of the line from where the constructor is called. + protected EoWrapper(IntPtr baseKlass, System.Type managedType, Efl.Object parent, + [CallerFilePath] string file = null, + [CallerLineNumber] int line = 0) + { + inherited = ((object)this).GetType() != managedType; + IntPtr actual_klass = baseKlass; + if (inherited) + { + actual_klass = Efl.Eo.ClassRegister.GetInheritKlassOrRegister(baseKlass, ((object)this).GetType()); + } + + // Creation of the unfinalized Eo handle + Eina.Log.Debug($"Instantiating from klass 0x{actual_klass.ToInt64():x}"); + System.IntPtr parent_ptr = System.IntPtr.Zero; + if (parent != null) + { + parent_ptr = parent.NativeHandle; + } + + handle = Efl.Eo.Globals._efl_add_internal_start(file, line, actual_klass, parent_ptr, 1, 0); + if (handle == System.IntPtr.Zero) + { + throw new Exception("Instantiation failed"); + } + + Eina.Log.Debug($"Eo instance right after internal_start 0x{handle.ToInt64():x} with refcount {Efl.Eo.Globals.efl_ref_count(handle)}"); + Eina.Log.Debug($"Parent was 0x{parent_ptr.ToInt64()}"); + + // Creation of wrapper supervisor + AddWrapperSupervisor(); + } + + /// Destructor. + ~EoWrapper() + { + Dispose(false); + } + + /// Pointer to the native instance. + public System.IntPtr NativeHandle + { + get { return handle; } + } + + /// Pointer to the native class description. + public abstract System.IntPtr NativeClass + { + get; + } + + /// Releases the underlying native instance. + protected virtual void Dispose(bool disposing) + { + if (disposing && handle != System.IntPtr.Zero) + { + IntPtr h = handle; + handle = IntPtr.Zero; + Efl.Eo.Globals.efl_mono_native_dispose(h); + } + else + { + Monitor.Enter(Efl.All.InitLock); + if (Efl.All.MainLoopInitialized) + { + Efl.Eo.Globals.efl_mono_thread_safe_native_dispose(handle); + } + + Monitor.Exit(Efl.All.InitLock); + } + } + + /// Turns the native pointer into a string representation. + /// A string with the type and the native pointer for this object. + public override String ToString() + { + return $"{this.GetType().Name}@[0x{(UInt64)handle:x}]"; + } + + /// Releases the underlying native instance. + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + /// Releases the underlying Eo object. + /// + /// This method is a C# counterpart to the C `efl_del` function. It removes the parent of the object + /// and releases the Eo reference it was holding. + /// + public void Del() + { + // FIXME Implement this + ((Efl.Object)this).SetParent(null); + Dispose(); + } + + /// Finishes instantiating this object. + /// Internal usage by generated code. + protected void FinishInstantiation() + { + Eina.Log.Debug("calling efl_add_internal_end"); + var h = Efl.Eo.Globals._efl_add_end(handle, 1, 0); + Eina.Log.Debug($"efl_add_end returned eo 0x{handle.ToInt64():x}"); + + // if (h == IntPtr.Zero) // TODO + // { + // } + + handle = h; + } + + /// Adds a new event handler, registering it to the native event. For internal use only. + /// The name of the native library definining the event. + /// The name of the native event. + /// Delegate to be called by native code on event raising. + /// Managed delegate that will be called by evtCaller on event raising. + protected void AddNativeEventHandler(string lib, string key, Efl.EventCb evtCaller, object evtDelegate) + { + IntPtr desc = Efl.EventDescription.GetNative(lib, key); + if (desc == IntPtr.Zero) + { + Eina.Log.Error($"Failed to get native event {key}"); + return; + } + + var wsPtr = Efl.Eo.Globals.efl_mono_wrapper_supervisor_get(handle); + var ws = Efl.Eo.Globals.WrapperSupervisorPtrToManaged(wsPtr); + if (ws.EoEvents.ContainsKey((desc, evtDelegate))) + { + Eina.Log.Warning($"Event proxy for event {key} already registered!"); + return; + } + + IntPtr evtCallerPtr = Marshal.GetFunctionPointerForDelegate(evtCaller); + if (!Efl.Eo.Globals.efl_event_callback_priority_add(handle, desc, 0, evtCallerPtr, wsPtr)) + { + Eina.Log.Error($"Failed to add event proxy for event {key}"); + return; + } + + ws.EoEvents[(desc, evtDelegate)] = (evtCallerPtr, evtCaller); + Eina.Error.RaiseIfUnhandledException(); + } + + /// Removes the given event handler for the given event. For internal use only. + /// The name of the native library definining the event. + /// The name of the native event. + /// The delegate to be removed. + protected void RemoveNativeEventHandler(string lib, string key, object evtDelegate) + { + IntPtr desc = Efl.EventDescription.GetNative(lib, key); + if (desc == IntPtr.Zero) + { + Eina.Log.Error($"Failed to get native event {key}"); + return; + } + + var wsPtr = Efl.Eo.Globals.efl_mono_wrapper_supervisor_get(handle); + var ws = Efl.Eo.Globals.WrapperSupervisorPtrToManaged(wsPtr); + var evtPair = (desc, evtDelegate); + if (ws.EoEvents.TryGetValue(evtPair, out var caller)) + { + if (!Efl.Eo.Globals.efl_event_callback_del(handle, desc, caller.evtCallerPtr, wsPtr)) + { + Eina.Log.Error($"Failed to remove event proxy for event {key}"); + return; + } + + ws.EoEvents.Remove(evtPair); + Eina.Error.RaiseIfUnhandledException(); + } + else + { + Eina.Log.Error($"Trying to remove proxy for event {key} when it is not registered."); + } + } + + private static void OwnershipUniqueCallback(IntPtr data, ref Efl.Event.NativeStruct evt) + { + var ws = Efl.Eo.Globals.WrapperSupervisorPtrToManaged(data); + ws.MakeUnique(); + } + + private static void OwnershipSharedCallback(IntPtr data, ref Efl.Event.NativeStruct evt) + { + var ws = Efl.Eo.Globals.WrapperSupervisorPtrToManaged(data); + ws.MakeShared(); + } + + /// Create and set to the internal native state a C# supervisor for this Eo wrapper. For internal use only. + private void AddWrapperSupervisor() + { + var ws = new Efl.Eo.WrapperSupervisor(this); + Efl.Eo.Globals.SetWrapperSupervisor(handle, ws); + if (Efl.Eo.Globals.efl_ref_count(handle) > 1) + { + ws.MakeShared(); + } + + AddOwnershipEventHandlers(); + } + + /// Register handlers to ownership events, in order to control the object lifetime. For internal use only. + private void AddOwnershipEventHandlers() + { + AddNativeEventHandler("eo", "_EFL_EVENT_INVALIDATE", ownershipUniqueDelegate, ownershipUniqueDelegate); + AddNativeEventHandler("eo", "_EFL_EVENT_OWNERSHIP_UNIQUE", ownershipUniqueDelegate, ownershipUniqueDelegate); + AddNativeEventHandler("eo", "_EFL_EVENT_OWNERSHIP_SHARED", ownershipSharedDelegate, ownershipSharedDelegate); + Eina.Error.RaiseIfUnhandledException(); + } +} + +} // namespace Global + +} // namespace Efl diff --git a/src/bindings/mono/eo_mono/WrapperSupervisor.cs b/src/bindings/mono/eo_mono/WrapperSupervisor.cs new file mode 100644 index 0000000000..21ef05268f --- /dev/null +++ b/src/bindings/mono/eo_mono/WrapperSupervisor.cs @@ -0,0 +1,64 @@ +using System; +using EventDictionary = System.Collections.Generic.Dictionary<(System.IntPtr desc, object evtDelegate), (System.IntPtr evtCallerPtr, Efl.EventCb evtCaller)>; + +namespace Efl +{ + +namespace Eo +{ + +/// Observe the ownership state of an Eo wrapper and control its life-cycle. +public class WrapperSupervisor +{ + private System.WeakReference weakRef; +#pragma warning disable CS0414 + private Efl.Eo.IWrapper sharedRef; +#pragma warning restore CS0414 + private EventDictionary eoEvents; + + /// Create a new supervisor for the given. + /// Efl object to be supervised. + public WrapperSupervisor(Efl.Eo.IWrapper obj) + { + weakRef = new WeakReference(obj); + sharedRef = null; + eoEvents = new EventDictionary(); + } + + /// Efl object being supervised. + public Efl.Eo.IWrapper Target + { + get + { + return (Efl.Eo.IWrapper) weakRef.Target; + } + } + + /// Dictionary that holds the events related with the supervised object. + public EventDictionary EoEvents + { + get + { + return eoEvents; + } + } + + /// To be called when the object is uniquely owned by C#, removing its strong reference and making it available to garbage collection. + public void MakeUnique() + { + sharedRef = null; + } + + /// To be called when the object is owned in the native library too, adding a strong reference to it and making it unavailable for garbage collection. + public void MakeShared() + { + if (this.Target == null) + throw new InvalidOperationException("Tried to make a null reference shared."); + sharedRef = this.Target; + } +} + +} + +} + diff --git a/src/bindings/mono/eo_mono/iwrapper.cs b/src/bindings/mono/eo_mono/iwrapper.cs index a410a091bb..9e73ca4687 100644 --- a/src/bindings/mono/eo_mono/iwrapper.cs +++ b/src/bindings/mono/eo_mono/iwrapper.cs @@ -48,6 +48,10 @@ public class Globals public delegate IntPtr _efl_add_internal_start_delegate([MarshalAs(UnmanagedType.LPStr)] String file, int line, IntPtr klass, IntPtr parent, byte is_ref, byte is_fallback); + + [DllImport(efl.Libs.CustomExports)] public static extern IntPtr efl_mono_wrapper_supervisor_get(IntPtr eo); + [DllImport(efl.Libs.CustomExports)] public static extern void efl_mono_wrapper_supervisor_set(IntPtr eo, IntPtr ws); + [DllImport(efl.Libs.Eo)] public static extern IntPtr _efl_add_internal_start([MarshalAs(UnmanagedType.LPStr)] String file, int line, IntPtr klass, IntPtr parent, byte is_ref, byte is_fallback); @@ -68,11 +72,11 @@ public class Globals [DllImport(efl.Libs.Eo)] public static extern int efl_ref_count(IntPtr eo); [DllImport(efl.Libs.CustomExports)] public static extern void - efl_mono_gchandle_callbacks_set(Efl.FreeGCHandleCb freeGCHandleCb, Efl.RemoveEventsCb removeEventsCb); + efl_mono_wrapper_supervisor_callbacks_set(Efl.FreeWrapperSupervisorCb freeWrapperSupervisorCb); [DllImport(efl.Libs.CustomExports)] public static extern void - efl_mono_native_dispose(IntPtr eo, IntPtr gcHandle); + efl_mono_native_dispose(IntPtr eo); [DllImport(efl.Libs.CustomExports)] public static extern void - efl_mono_thread_safe_native_dispose(IntPtr eo, IntPtr gcHandle); + efl_mono_thread_safe_native_dispose(IntPtr eo); [DllImport(efl.Libs.CustomExports)] public static extern void efl_mono_thread_safe_efl_unref(IntPtr eo); @@ -231,7 +235,7 @@ public class Globals description.version = 2; // EO_VERSION description.name = class_name; description.class_type = 0; // REGULAR - description.data_size = (UIntPtr)8; + description.data_size = (UIntPtr)0; description.class_initializer = IntPtr.Zero; description.class_constructor = IntPtr.Zero; description.class_destructor = IntPtr.Zero; @@ -246,6 +250,8 @@ public class Globals IntPtr description_ptr = Eina.MemoryNative.Alloc(Marshal.SizeOf(description)); Marshal.StructureToPtr(description, description_ptr, false); + // FIXME: description_ptr seems to be leaking memory even after an eo_shutdown + var interface_list = EoG.get_efl_interfaces(type); Eina.Log.Debug($"Going to register new class named {class_name}"); @@ -442,60 +448,26 @@ public class Globals } } - public static IntPtr instantiate_start(IntPtr klass, Efl.Object parent, - [CallerFilePath] string file = null, - [CallerLineNumber] int line = 0) + public static Efl.Eo.WrapperSupervisor WrapperSupervisorPtrToManaged(IntPtr wsPtr) { - Eina.Log.Debug($"Instantiating from klass 0x{klass.ToInt64():x}"); - System.IntPtr parent_ptr = System.IntPtr.Zero; - if (parent != null) - { - parent_ptr = parent.NativeHandle; - } - - 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"); - } - - Eina.Log.Debug($"Eo instance right after internal_start 0x{eo.ToInt64():x} with refcount {Efl.Eo.Globals.efl_ref_count(eo)}"); - Eina.Log.Debug($"Parent was 0x{parent_ptr.ToInt64()}"); - return eo; + return (Efl.Eo.WrapperSupervisor) GCHandle.FromIntPtr(wsPtr).Target; } - public static IntPtr instantiate_end(IntPtr eo) + public static Efl.Eo.WrapperSupervisor GetWrapperSupervisor(IntPtr eo) { - Eina.Log.Debug("calling efl_add_internal_end"); - eo = Efl.Eo.Globals._efl_add_end(eo, 1, 0); - Eina.Log.Debug($"efl_add_end returned eo 0x{eo.ToInt64():x}"); - return eo; - } - - public static void PrivateDataSet(Efl.Eo.IWrapper obj) - { - Eina.Log.Debug($"Calling data_scope_get with obj {obj.NativeHandle.ToInt64():x} and klass {obj.NativeClass.ToInt64():x}"); - IntPtr pd = Efl.Eo.Globals.efl_data_scope_get(obj.NativeHandle, obj.NativeClass); - { - GCHandle gch = GCHandle.Alloc(obj); - EolianPD epd; - epd.pointer = GCHandle.ToIntPtr(gch); - Marshal.StructureToPtr(epd, pd, false); - } - } - - public static Efl.Eo.IWrapper PrivateDataGet(IntPtr pd) - { - EolianPD epd = (EolianPD)Marshal.PtrToStructure(pd, typeof(EolianPD)); - if (epd.pointer != IntPtr.Zero) - { - GCHandle gch = GCHandle.FromIntPtr(epd.pointer); - return (Efl.Eo.IWrapper)gch.Target; - } - else + var wsPtr = Efl.Eo.Globals.efl_mono_wrapper_supervisor_get(eo); + if (wsPtr == IntPtr.Zero) { return null; } + + return WrapperSupervisorPtrToManaged(wsPtr); + } + + public static void SetWrapperSupervisor(IntPtr eo, Efl.Eo.WrapperSupervisor ws) + { + GCHandle gch = GCHandle.Alloc(ws); + Efl.Eo.Globals.efl_mono_wrapper_supervisor_set(eo, GCHandle.ToIntPtr(gch)); } public static void free_dict_values(Dictionary dict) @@ -601,93 +573,101 @@ public class Globals return null; } - IntPtr eoKlass = efl_class_get(handle); - - if (eoKlass == IntPtr.Zero) + Efl.Eo.Globals.efl_ref(handle); + try { - throw new InvalidOperationException($"Can't get Eo class for object handle 0x{handle.ToInt64():x}"); - } - - var managedType = ClassRegister.GetManagedType(eoKlass); - - if (managedType == null) - { - IntPtr nativeName = efl_class_name_get(eoKlass); - var name = Eina.StringConversion.NativeUtf8ToManagedString(nativeName); - - throw new InvalidOperationException($"Can't get Managed class for object handle 0x{handle.ToInt64():x} with native class [{name}]"); - } - - // Pure C# classes that inherit from generated classes store their C# instance in their - // Eo private data field. - if (!IsGeneratedClass(managedType)) - { - Efl.Eo.IWrapper instance = null; - IntPtr pd = efl_data_scope_get(handle, eoKlass); - - if (pd != IntPtr.Zero) + var ws = Efl.Eo.Globals.GetWrapperSupervisor(handle); + if (ws != null && ws.Target != null) { - instance = PrivateDataGet(pd); + if (!shouldIncRef) + { + Efl.Eo.Globals.efl_unref(handle); + } + + return ws.Target; } - return instance; - } + IntPtr eoKlass = efl_class_get(handle); - System.Reflection.ConstructorInfo constructor = null; - - try - { - var flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; - constructor = managedType.GetConstructor(flags, null, new Type[1] { typeof(System.IntPtr) }, null); - } - catch (InvalidOperationException) - { - throw new InvalidOperationException($"Can't get constructor for type {managedType}"); - } - - var ret = constructor.Invoke(new object[1] { handle }) as Efl.Eo.IWrapper; - - if (ret != null && shouldIncRef) - Efl.Eo.Globals.efl_ref(handle); - - return ret; - } - - private static Efl.FreeGCHandleCb FreeGCHandleCallbackDelegate = new Efl.FreeGCHandleCb(FreeGCHandleCallback); - public static void FreeGCHandleCallback(IntPtr gcHandlePtr) - { - try - { - GCHandle gcHandle = GCHandle.FromIntPtr(gcHandlePtr); - gcHandle.Free(); - } - catch (Exception e) - { - Eina.Log.Error(e.ToString()); - Eina.Error.Set(Eina.Error.UNHANDLED_EXCEPTION); - } - } - - private static Efl.RemoveEventsCb RemoveEventsCallbackDelegate = new Efl.RemoveEventsCb(RemoveEventsCallback); - public static void RemoveEventsCallback(IntPtr obj, IntPtr gcHandlePtr) - { - try - { - GCHandle gcHandle = GCHandle.FromIntPtr(gcHandlePtr); - var eoEvents = gcHandle.Target as Dictionary<(IntPtr desc, object evtDelegate), (IntPtr evtCallerPtr, Efl.EventCb evtCaller)>; - if (eoEvents == null) + if (eoKlass == IntPtr.Zero) { - Eina.Log.Error($"Invalid event dictionary [GCHandle pointer: {gcHandlePtr}]"); + throw new InvalidOperationException($"Can't get Eo class for object handle 0x{handle.ToInt64():x}"); + } + + var managedType = ClassRegister.GetManagedType(eoKlass); + + if (managedType == null) + { + IntPtr nativeName = efl_class_name_get(eoKlass); + var name = Eina.StringConversion.NativeUtf8ToManagedString(nativeName); + + throw new InvalidOperationException($"Can't get Managed class for object handle 0x{handle.ToInt64():x} with native class [{name}]"); + } + + Debug.Assert(IsGeneratedClass(managedType)); + System.Reflection.ConstructorInfo constructor = null; + + try + { + var flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; + constructor = managedType.GetConstructor(flags, null, new Type[1] { typeof(System.IntPtr) }, null); + } + catch (InvalidOperationException) + { + throw new InvalidOperationException($"Can't get constructor for type {managedType}"); + } + + var ret = (Efl.Eo.IWrapper) constructor.Invoke(new object[1] { handle }); + + if (ret == null) + { + throw new InvalidOperationException($"Can't construct type {managedType} from IntPtr handle"); + } + + if (shouldIncRef) + { + Efl.Eo.Globals.efl_ref(handle); + } + + return ret; + } + finally + { + Efl.Eo.Globals.efl_unref(handle); + } + } + + private static Efl.FreeWrapperSupervisorCb FreeWrapperSupervisorCallbackDelegate = new Efl.FreeWrapperSupervisorCb(FreeWrapperSupervisorCallback); + public static void FreeWrapperSupervisorCallback(IntPtr eo) + { + try + { + var wsPtr = Efl.Eo.Globals.efl_mono_wrapper_supervisor_get(eo); + if (wsPtr == IntPtr.Zero) + { + Eina.Log.Error($"Invalid wrapper supervisor [Eo pointer: {eo.ToInt64():x}]"); return; } - foreach (var item in eoEvents) + Efl.Eo.Globals.efl_mono_wrapper_supervisor_set(eo, IntPtr.Zero); + + GCHandle gch = GCHandle.FromIntPtr(wsPtr); + var ws = (Efl.Eo.WrapperSupervisor) gch.Target; + foreach (var item in ws.EoEvents) { - if (!efl_event_callback_del(obj, item.Key.desc, item.Value.evtCallerPtr, IntPtr.Zero)) + if (!efl_event_callback_del(eo, item.Key.desc, item.Value.evtCallerPtr, wsPtr)) { - Eina.Log.Error($"Failed to remove event proxy for event {item.Key.desc} [cb: {item.Value.evtCallerPtr}]"); + Eina.Log.Error($"Failed to remove event proxy for event {item.Key.desc} [eo: {eo.ToInt64():x}; cb: {item.Value.evtCallerPtr.ToInt64():x}]"); } } + + // Free the native eo + Efl.Eo.Globals.efl_unref(eo); + + // now the WrapperSupervisor can be collected, and so its member: + // - the event dictionary + // - and the EoWrapper if it is still pinned + gch.Free(); } catch (Exception e) { @@ -698,7 +678,7 @@ public class Globals public static void SetNativeDisposeCallbacks() { - efl_mono_gchandle_callbacks_set(FreeGCHandleCallbackDelegate, RemoveEventsCallbackDelegate); + efl_mono_wrapper_supervisor_callbacks_set(FreeWrapperSupervisorCallbackDelegate); } public static void ThreadSafeFreeCbExec(EinaFreeCb cbFreeCb, IntPtr cbData) @@ -800,8 +780,6 @@ public static class ClassRegister string name = Eina.StringConversion.NativeUtf8ToManagedString(namePtr) .Replace("_", ""); // Convert Efl C name to C# name - var klass_type = Efl.Eo.Globals.efl_class_type_get(klass); - // Check if this is an internal implementation of an abstract class var abstract_impl_suffix = "Realized"; if (name.EndsWith(abstract_impl_suffix)) @@ -813,6 +791,7 @@ public static class ClassRegister } // When converting to managed, interfaces and mixins gets the 'I' prefix. + var klass_type = Efl.Eo.Globals.efl_class_type_get(klass); if (klass_type == Efl.Eo.Globals.EflClassType.Interface || klass_type == Efl.Eo.Globals.EflClassType.Mixin) { var pos = name.LastIndexOf("."); diff --git a/src/bindings/mono/eo_mono/meson.build b/src/bindings/mono/eo_mono/meson.build index 8aca4004df..013b1cc97f 100644 --- a/src/bindings/mono/eo_mono/meson.build +++ b/src/bindings/mono/eo_mono/meson.build @@ -2,7 +2,9 @@ mono_files += files( 'iwrapper.cs', 'workaround.cs', 'FunctionWrapper.cs', - 'NativeModule.cs' + 'NativeModule.cs', + 'EoWrapper.cs', + 'WrapperSupervisor.cs' ) if host_machine.system() == 'windows' diff --git a/src/bindings/mono/eo_mono/workaround.cs b/src/bindings/mono/eo_mono/workaround.cs index e32c921862..9f22b90fa6 100644 --- a/src/bindings/mono/eo_mono/workaround.cs +++ b/src/bindings/mono/eo_mono/workaround.cs @@ -44,12 +44,6 @@ public struct Efl_Object_Ops public UIntPtr count; }; -[StructLayout(LayoutKind.Sequential)] -public struct EolianPD -{ - public IntPtr pointer; -} - #pragma warning disable 0169 public struct EvasObjectBoxLayout @@ -115,8 +109,7 @@ public struct EventDescription }; public delegate void EventCb(System.IntPtr data, ref Event.NativeStruct evt); -public delegate void FreeGCHandleCb(System.IntPtr gcHandle); -public delegate void RemoveEventsCb(System.IntPtr obj, System.IntPtr gcHandle); +public delegate void FreeWrapperSupervisorCb(System.IntPtr obj); [StructLayout(LayoutKind.Sequential)] public struct TextCursorCursor diff --git a/src/lib/efl_mono/efl_custom_exports_mono.c b/src/lib/efl_mono/efl_custom_exports_mono.c index c4a3b54bc5..55f0054da0 100644 --- a/src/lib/efl_mono/efl_custom_exports_mono.c +++ b/src/lib/efl_mono/efl_custom_exports_mono.c @@ -23,44 +23,39 @@ # endif #endif /* ! _WIN32 */ -typedef void (*Efl_Mono_Free_GCHandle_Cb)(void *gchandle); -typedef void (*Efl_Mono_Remove_Events_Cb)(Eo *obj, void *gchandle); -static Efl_Mono_Free_GCHandle_Cb _efl_mono_free_gchandle_call = NULL; -static Efl_Mono_Remove_Events_Cb _efl_mono_remove_events_call = NULL; - -EAPI void efl_mono_gchandle_callbacks_set(Efl_Mono_Free_GCHandle_Cb free_gchandle_cb, Efl_Mono_Remove_Events_Cb remove_events_cb) +EAPI const char *efl_mono_wrapper_supervisor_key_get() { - _efl_mono_free_gchandle_call = free_gchandle_cb; - _efl_mono_remove_events_call = remove_events_cb; + return "__c#_wrapper_supervisor"; } -EAPI void efl_mono_native_dispose(Eo *obj, void* gchandle) +EAPI void *efl_mono_wrapper_supervisor_get(Eo *eo) { - if (gchandle) _efl_mono_remove_events_call(obj, gchandle); - efl_unref(obj); - if (gchandle) _efl_mono_free_gchandle_call(gchandle); + return efl_key_data_get(eo, efl_mono_wrapper_supervisor_key_get()); } -typedef struct _Efl_Mono_Native_Dispose_Data +EAPI void efl_mono_wrapper_supervisor_set(Eo *eo, void *ws) { - Eo *obj; - void *gchandle; -} Efl_Mono_Native_Dispose_Data; - -static void _efl_mono_native_dispose_cb(void *data) -{ - Efl_Mono_Native_Dispose_Data *dd = data; - efl_mono_native_dispose(dd->obj, dd->gchandle); - free(dd); + efl_key_data_set(eo, efl_mono_wrapper_supervisor_key_get(), ws); } -EAPI void efl_mono_thread_safe_native_dispose(Eo *obj, void* gchandle) +typedef void (*Efl_Mono_Free_Wrapper_Supervisor_Cb)(Eo *obj); + +static Efl_Mono_Free_Wrapper_Supervisor_Cb _efl_mono_free_wrapper_supervisor_call = NULL; + +EAPI void efl_mono_wrapper_supervisor_callbacks_set(Efl_Mono_Free_Wrapper_Supervisor_Cb free_wrapper_supervisor_cb) { - Efl_Mono_Native_Dispose_Data *dd = malloc(sizeof(Efl_Mono_Native_Dispose_Data)); - dd->obj = obj; - dd->gchandle = gchandle; - ecore_main_loop_thread_safe_call_async(_efl_mono_native_dispose_cb, dd); + _efl_mono_free_wrapper_supervisor_call = free_wrapper_supervisor_cb; +} + +EAPI void efl_mono_native_dispose(Eo *obj) +{ + _efl_mono_free_wrapper_supervisor_call(obj); +} + +EAPI void efl_mono_thread_safe_native_dispose(Eo *obj) +{ + ecore_main_loop_thread_safe_call_async((Ecore_Cb)efl_mono_native_dispose, obj); } static void _efl_mono_unref_cb(void *obj) diff --git a/src/tests/efl_mono/Eo.cs b/src/tests/efl_mono/Eo.cs index 95b6b6395e..be66f0842c 100644 --- a/src/tests/efl_mono/Eo.cs +++ b/src/tests/efl_mono/Eo.cs @@ -510,4 +510,19 @@ class TestProvider } } +class TestObjectDeletion +{ + public static void test_object_deletion() + { + var obj = new Dummy.PartHolder(); + var part = obj.OnePart; + + Test.AssertNotNull(part); + + part.Del(); + + Test.AssertNull(obj.OnePart); + } +} + } diff --git a/src/tests/efl_mono/Inheritance.cs b/src/tests/efl_mono/Inheritance.cs index befdd3a7b4..74c1086e78 100644 --- a/src/tests/efl_mono/Inheritance.cs +++ b/src/tests/efl_mono/Inheritance.cs @@ -35,6 +35,46 @@ class TestInheritance } } + internal class Inherit3Parent : Dummy.TestObject + { + public bool disposed = false; + public bool childDisposed = false; + + ~Inherit3Parent() + { + Console.WriteLine("finalizer called for parent"); + } + + protected override void Dispose(bool disposing) + { + Console.WriteLine("Dispose parent"); + base.Dispose(disposing); + } + } + + internal class Inherit3Child : Dummy.TestObject + { + Inherit3Parent parent; + public Inherit3Child(Inherit3Parent parent) : base(parent) + { + // WARNING: Uncommenting the line below causes the parent-child cycle to leak. + // The GC won't be able to collect it. + // this.parent = parent; + } + + ~Inherit3Child() + { + Console.WriteLine("finalizer called for child"); + } + + protected override void Dispose(bool disposing) + { + /* parent.childDisposed = true; */ + Console.WriteLine("Dispose parent"); + base.Dispose(disposing); + } + } + public static void test_inherit_from_regular_class() { var obj = new Inherit1(); @@ -50,6 +90,55 @@ class TestInheritance string s = Dummy.InheritHelper.ReceiveDummyAndCallInStringshare(obj); Test.AssertEquals ("Hello World", s); } + + private static void CreateAndCheckInheritedObjects(out WeakReference parentWRef, out WeakReference childWRef) + { + var parent = new Inherit3Parent(); + var child = new Inherit3Child(parent); + + parentWRef = new WeakReference(parent); + childWRef = new WeakReference(child); + + Console.WriteLine($"Parent [{parent.ToString()}] has {Efl.Eo.Globals.efl_ref_count(parent.NativeHandle)} refs"); + Console.WriteLine($"Child [{child.ToString()}] has {Efl.Eo.Globals.efl_ref_count(child.NativeHandle)} refs"); + + child = null; + + System.GC.Collect(System.GC.MaxGeneration, GCCollectionMode.Forced, true, true); + System.GC.WaitForPendingFinalizers(); + Efl.App.AppMain.Iterate(); + + child = (Inherit3Child) childWRef.Target; + + Test.AssertNotNull(parent); + Test.AssertNotNull(child); + Test.AssertEquals(false, parent.disposed); + Test.AssertEquals(false, parent.childDisposed); + + Console.WriteLine($"Parent [{parent.ToString()}] has {Efl.Eo.Globals.efl_ref_count(parent.NativeHandle)} refs"); + Console.WriteLine($"Child [{child.ToString()}] has {Efl.Eo.Globals.efl_ref_count(child.NativeHandle)} refs"); + + parent = null; + child = null; + } + + public static void test_inherit_lifetime() + { + WeakReference parentWRef; + WeakReference childWRef; + + CreateAndCheckInheritedObjects(out parentWRef, out childWRef); + + // Two invocations to iterate a the child wasn't being released with a single one + Test.CollectAndIterate(); + Test.CollectAndIterate(); + + var parent = (Dummy.TestObject) parentWRef.Target; + var child = (Dummy.TestObject) childWRef.Target; + + Test.AssertNull(parent); + Test.AssertNull(child); + } } } diff --git a/src/tests/efl_mono/TestUtils.cs b/src/tests/efl_mono/TestUtils.cs index 657bdb0300..b66f15a0bc 100644 --- a/src/tests/efl_mono/TestUtils.cs +++ b/src/tests/efl_mono/TestUtils.cs @@ -43,13 +43,14 @@ public static class Test [CallerFilePath] string file = null, [CallerMemberName] string member = null) { - if (file == null) - file = "(unknown file)"; - if (member == null) - member = "(unknown member)"; - if (expected == null) - throw new AssertionException($"{file}:{line} ({member}) Null expected value. Use AssertNull."); - if (!expected.Equals(actual)) { + if (expected == null && actual == null) + return; + if (expected == null || !expected.Equals(actual)) + { + if (file == null) + file = "(unknown file)"; + if (member == null) + member = "(unknown member)"; if (msg == null || msg.Length == 0) msg = $"Expected \"{expected}\", actual \"{actual}\""; throw new AssertionException($"{file}:{line} ({member}) {msg}"); @@ -62,13 +63,12 @@ public static class Test [CallerFilePath] string file = null, [CallerMemberName] string member = null) { - if (file == null) - file = "(unknown file)"; - if (member == null) - member = "(unknown member)"; - if (expected == null) - throw new AssertionException($"{file}:{line} ({member}) Null expected value. Use AssertNull."); - if (expected.Equals(actual)) { + if (expected == null ? actual == null : expected.Equals(actual)) + { + if (file == null) + file = "(unknown file)"; + if (member == null) + member = "(unknown member)"; if (msg == null || msg.Length == 0) msg = $"Expected \"{expected}\" shouldn't be equal to actual \"{actual}\""; throw new AssertionException($"{file}:{line} ({member}) {msg}"); diff --git a/src/tests/efl_mono/dummy_part_holder.c b/src/tests/efl_mono/dummy_part_holder.c index b595ac1f8d..aaeee6cec8 100644 --- a/src/tests/efl_mono/dummy_part_holder.c +++ b/src/tests/efl_mono/dummy_part_holder.c @@ -6,6 +6,16 @@ typedef struct Dummy_Part_Holder_Data Eo *two; } Dummy_Part_Holder_Data; +void part_deleted_cb(void *data, const Efl_Event *evt) +{ + Dummy_Part_Holder_Data *pd = data; + + if (evt->object == pd->one) + pd->one = NULL; + else if (evt->object == pd->two) + pd->two = NULL; +} + // Part holder static Efl_Object* _dummy_part_holder_efl_object_constructor(Eo *obj, Dummy_Part_Holder_Data *pd) @@ -16,12 +26,25 @@ _dummy_part_holder_efl_object_constructor(Eo *obj, Dummy_Part_Holder_Data *pd) if (!efl_parent_get(obj)) { pd->one = efl_add(DUMMY_TEST_OBJECT_CLASS, obj, efl_name_set(efl_added, "part_one")); + efl_event_callback_add(pd->one, EFL_EVENT_DEL, part_deleted_cb, pd); pd->two = efl_add(DUMMY_TEST_OBJECT_CLASS, obj, efl_name_set(efl_added, "part_two")); + efl_event_callback_add(pd->two, EFL_EVENT_DEL, part_deleted_cb, pd); + + } return obj; } +static void +_dummy_part_holder_efl_object_destructor(EINA_UNUSED Eo* obj, Dummy_Part_Holder_Data *pd) +{ + if (pd->one) + efl_parent_set(pd->one, NULL); + if (pd->two) + efl_parent_set(pd->two, NULL); +} + Efl_Object *_dummy_part_holder_efl_part_part_get(EINA_UNUSED const Eo *obj, Dummy_Part_Holder_Data *pd, const char *name) { if (!strcmp(name, "one")) diff --git a/src/tests/efl_mono/dummy_part_holder.eo b/src/tests/efl_mono/dummy_part_holder.eo index d65fd12e5b..b95f1a5cdc 100644 --- a/src/tests/efl_mono/dummy_part_holder.eo +++ b/src/tests/efl_mono/dummy_part_holder.eo @@ -9,5 +9,6 @@ class @beta Dummy.Part_Holder extends Dummy.Test_Object implements Efl.Part { implements { Efl.Part.part_get; Efl.Object.constructor; + Efl.Object.destructor; } }