EventObject no longer used for static events. EventBinding is constru…

…cted directly instead. Also fixes event_rename domain reload test case
losttech · lostmsu · Dec 9, 2021 · Oct 24, 2021 · Oct 28, 2021 · Oct 28, 2021
commit b1c9f5b0430ed2034c07eb7cdd012ed11f52d9f1
diff --git a/src/runtime/EventHandlerCollection.cs b/src/runtime/EventHandlerCollection.cs
@@ -0,0 +1,105 @@
+using System;
+using System.Collections.Generic;
+using System.Reflection;
+
+namespace Python.Runtime;
+
+internal class EventHandlerCollection: Dictionary<object, List<Handler>>
+{
+    readonly EventInfo info;
+    public EventHandlerCollection(EventInfo @event)
+    {
+        info = @event;
+    }
+
+    /// <summary>
+    /// Register a new Python object event handler with the event.
+    /// </summary>
+    internal bool AddEventHandler(BorrowedReference target, PyObject handler)
+    {
+        object? obj = null;
+        if (target != null)
+        {
+            var co = (CLRObject)ManagedType.GetManagedObject(target)!;
+            obj = co.inst;
+        }
+
+        // Create a true delegate instance of the appropriate type to
+        // wrap the Python handler. Note that wrapper delegate creation
+        // always succeeds, though calling the wrapper may fail.
+        Type type = info.EventHandlerType;
+        Delegate d = PythonEngine.DelegateManager.GetDelegate(type, handler);
+
+        // Now register the handler in a mapping from instance to pairs
+        // of (handler hash, delegate) so we can lookup to remove later.
+        object key = obj ?? info.ReflectedType;
+        if (!TryGetValue(key, out var list))
+        {
+            list = new List<Handler>();
+            this[key] = list;
+        }
+        list.Add(new Handler(Runtime.PyObject_Hash(handler), d));
+
+        // Note that AddEventHandler helper only works for public events,
+        // so we have to get the underlying add method explicitly.
+        object[] args = { d };
+        MethodInfo mi = info.GetAddMethod(true);
+        mi.Invoke(obj, BindingFlags.Default, null, args, null);
+
+        return true;
+    }
+
+
+    /// <summary>
+    /// Remove the given Python object event handler.
+    /// </summary>
+    internal bool RemoveEventHandler(BorrowedReference target, BorrowedReference handler)
+    {
+        object? obj = null;
+        if (target != null)
+        {
+            var co = (CLRObject)ManagedType.GetManagedObject(target)!;
+            obj = co.inst;
+        }
+
+        nint hash = Runtime.PyObject_Hash(handler);
+        if (hash == -1 && Exceptions.ErrorOccurred())
+        {
+            return false;
+        }
+
+        object key = obj ?? info.ReflectedType;
+
+        if (!TryGetValue(key, out var list))
+        {
+            Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
+            return false;
+        }
+
+        object?[] args = { null };
+        MethodInfo mi = info.GetRemoveMethod(true);
+
+        for (var i = 0; i < list.Count; i++)
+        {
+            var item = (Handler)list[i];
+            if (item.hash != hash)
+            {
+                continue;
+            }
+            args[0] = item.del;
+            try
+            {
+                mi.Invoke(obj, BindingFlags.Default, null, args, null);
+            }
+            catch
+            {
+                continue;
+            }
+            list.RemoveAt(i);
+            return true;
+        }
+
+        Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
+        return false;
+    }
+}
diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs
@@ -491,7 +491,9 @@ private static ClassInfo GetClassInfo(Type type)
                         {
                             continue;
                         }
-                        ob = new EventObject(ei);
+                        ob = ei.AddMethod.IsStatic
+                            ? new EventBinding(ei)
+                            : new EventObject(ei);
                         ci.members[ei.Name] = ob;
                         continue;
 

diff --git a/src/runtime/eventbinding.cs b/src/runtime/eventbinding.cs
@@ -1,4 +1,6 @@
 using System;
+using System.Diagnostics;
+using System.Reflection;
 
 namespace Python.Runtime
 {
@@ -8,15 +10,22 @@ namespace Python.Runtime
     [Serializable]
     internal class EventBinding : ExtensionType
     {
-        private EventObject e;
+        private readonly string name;
+        private readonly EventHandlerCollection e;
         private PyObject? target;
 
-        public EventBinding(EventObject e, PyObject? target)
+        public EventBinding(string name, EventHandlerCollection e, PyObject? target)
         {
+            this.name = name;
             this.target = target;
             this.e = e;
         }
 
+        public EventBinding(EventInfo @event) : this(@event.Name, new EventHandlerCollection(@event), target: null)
+        {
+            Debug.Assert(@event.AddMethod.IsStatic);
+        }
+
 
         /// <summary>
         /// EventBinding += operator implementation.
@@ -61,6 +70,10 @@ public static NewReference nb_inplace_subtract(BorrowedReference ob, BorrowedRef
             return new NewReference(ob);
         }
 
+        /// </summary>
+        public static int tp_descr_set(BorrowedReference ds, BorrowedReference ob, BorrowedReference val)
+            => EventObject.tp_descr_set(ds, ob, val);
+
 
         /// <summary>
         /// EventBinding  __hash__ implementation.
@@ -91,7 +104,7 @@ public static NewReference tp_repr(BorrowedReference ob)
         {
             var self = (EventBinding)GetManagedObject(ob)!;
             string type = self.target == null ? "unbound" : "bound";
-            string s = string.Format("<{0} event '{1}'>", type, self.e.name);
+            string s = string.Format("<{0} event '{1}'>", type, self.name);
             return Runtime.PyString_FromString(s);
         }
     }

diff --git a/src/runtime/eventobject.cs b/src/runtime/eventobject.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections;
+using System.Diagnostics;
 using System.Reflection;
 
 namespace Python.Runtime
@@ -10,124 +11,16 @@ namespace Python.Runtime
     [Serializable]
     internal class EventObject : ExtensionType
     {
-        internal string name;
-        internal PyObject? unbound;
-        internal EventInfo info;
-        internal Hashtable? reg;
+        internal readonly string name;
+        internal readonly EventHandlerCollection reg;
 
         public EventObject(EventInfo info)
         {
+            Debug.Assert(!info.AddMethod.IsStatic);
             this.name = info.Name;
-            this.info = info;
+            this.reg = new EventHandlerCollection(info);
         }
 
-
-        /// <summary>
-        /// Register a new Python object event handler with the event.
-        /// </summary>
-        internal bool AddEventHandler(BorrowedReference target, PyObject handler)
-        {
-            object? obj = null;
-            if (target != null)
-            {
-                var co = (CLRObject)GetManagedObject(target)!;
-                obj = co.inst;
-            }
-
-            // Create a true delegate instance of the appropriate type to
-            // wrap the Python handler. Note that wrapper delegate creation
-            // always succeeds, though calling the wrapper may fail.
-            Type type = info.EventHandlerType;
-            Delegate d = PythonEngine.DelegateManager.GetDelegate(type, handler);
-
-            // Now register the handler in a mapping from instance to pairs
-            // of (handler hash, delegate) so we can lookup to remove later.
-            // All this is done lazily to avoid overhead until an event is
-            // actually subscribed to by a Python event handler.
-            if (reg == null)
-            {
-                reg = new Hashtable();
-            }
-            object key = obj ?? info.ReflectedType;
-            var list = reg[key] as ArrayList;
-            if (list == null)
-            {
-                list = new ArrayList();
-                reg[key] = list;
-            }
-            list.Add(new Handler(Runtime.PyObject_Hash(handler), d));
-
-            // Note that AddEventHandler helper only works for public events,
-            // so we have to get the underlying add method explicitly.
-            object[] args = { d };
-            MethodInfo mi = info.GetAddMethod(true);
-            mi.Invoke(obj, BindingFlags.Default, null, args, null);
-
-            return true;
-        }
-
-
-        /// <summary>
-        /// Remove the given Python object event handler.
-        /// </summary>
-        internal bool RemoveEventHandler(BorrowedReference target, BorrowedReference handler)
-        {
-            if (reg == null)
-            {
-                Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
-                return false;
-            }
-
-            object? obj = null;
-            if (target != null)
-            {
-                var co = (CLRObject)GetManagedObject(target)!;
-                obj = co.inst;
-            }
-
-            nint hash = Runtime.PyObject_Hash(handler);
-            if (hash == -1 && Exceptions.ErrorOccurred())
-            {
-                return false;
-            }
-
-            object key = obj ?? info.ReflectedType;
-            var list = reg[key] as ArrayList;
-
-            if (list == null)
-            {
-                Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
-                return false;
-            }
-
-            object?[] args = { null };
-            MethodInfo mi = info.GetRemoveMethod(true);
-
-            for (var i = 0; i < list.Count; i++)
-            {
-                var item = (Handler)list[i];
-                if (item.hash != hash)
-                {
-                    continue;
-                }
-                args[0] = item.del;
-                try
-                {
-                    mi.Invoke(obj, BindingFlags.Default, null, args, null);
-                }
-                catch
-                {
-                    continue;
-                }
-                list.RemoveAt(i);
-                return true;
-            }
-
-            Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
-            return false;
-        }
-
-
         /// <summary>
         /// Descriptor __get__ implementation. A getattr on an event returns
         /// a "bound" event that keeps a reference to the object instance.
@@ -141,25 +34,17 @@ public static NewReference tp_descr_get(BorrowedReference ds, BorrowedReference
                 return Exceptions.RaiseTypeError("invalid argument");
             }
 
-            // If the event is accessed through its type (rather than via
-            // an instance) we return an 'unbound' EventBinding that will
-            // be cached for future accesses through the type.
-
             if (ob == null)
             {
-                if (self.unbound == null)
-                {
-                    self.unbound = new EventBinding(self, target: null).Alloc().MoveToPyObject();
-                }
-                return new NewReference(self.unbound);
+                return new NewReference(ds);
             }
 
             if (Runtime.PyObject_IsInstance(ob, tp) < 1)
             {
                 return Exceptions.RaiseTypeError("invalid argument");
             }
 
-            return new EventBinding(self, new PyObject(ob)).Alloc();
+            return new EventBinding(self.name, self.reg, new PyObject(ob)).Alloc();
         }
 
 
@@ -192,13 +77,6 @@ public static NewReference tp_repr(BorrowedReference ob)
             var self = (EventObject)GetManagedObject(ob)!;
             return Runtime.PyString_FromString($"<event '{self.name}'>");
         }
-
-
-        protected override void Clear(BorrowedReference ob)
-        {
-            this.unbound = null!;
-            base.Clear(ob);
-        }
     }
 
 

diff --git a/tests/domain_tests/TestRunner.cs b/tests/domain_tests/TestRunner.cs
@@ -310,7 +310,7 @@ public class Cls
                             public static event Action Before;
                             public static void Call()
                             {
-                                Before();
+                                if (Before != null) Before();
                             }
                         }
                     }",
@@ -324,7 +324,7 @@ public class Cls
                             public static event Action After;
                             public static void Call()
                             {
-                                After();
+                                if (After != null) After();
                             }
                         }
                     }",
@@ -335,21 +335,29 @@ import sys
 from TestNamespace import Cls
 
 called = False
+before_reload_called = False
+after_reload_called = False
 
 def callback_function():
     global called
     called = True
 
 def before_reload():
-    global called
+    global called, before_reload_called
     called = False
     Cls.Before += callback_function
     Cls.Call()
     assert called is True
+    before_reload_called = True
 
 def after_reload():
-    global called
+    global called, after_reload_called, before_reload_called
+
+    assert before_reload_called is True
+    if not after_reload_called:
     assert called is True
+    after_reload_called = True
+
     called = False
     Cls.Call()
     assert called is False