From 9dbea9a14e77ace86920e28f710df99c45113149 Mon Sep 17 00:00:00 2001
From: Victor Nova <lost@losttech.software>
Date: Sat, 22 May 2021 22:39:57 -0700
Subject: [PATCH 1/2] refactored tp_clear in ExtensionType and descendants into
 a virtual C# function Clear

---
 src/runtime/constructorbinding.cs | 14 ++++++--------
 src/runtime/eventbinding.cs       |  7 +++----
 src/runtime/eventobject.cs        | 10 ++++++++++
 src/runtime/extensiontype.cs      | 13 ++++++++++---
 src/runtime/methodbinding.cs      |  7 +++----
 src/runtime/methodobject.cs       |  9 ++++-----
 src/runtime/moduleobject.cs       | 25 ++++++++++++-------------
 src/runtime/overload.cs           |  6 ++++++
 8 files changed, 54 insertions(+), 37 deletions(-)

diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs
index ab4a7af0a..da35aafd1 100644
--- a/src/runtime/constructorbinding.cs
+++ b/src/runtime/constructorbinding.cs
@@ -155,11 +155,10 @@ protected override void Dealloc()
             base.Dealloc();
         }
 
-        public static int tp_clear(IntPtr ob)
+        protected override void Clear()
         {
-            var self = (ConstructorBinding)GetManagedObject(ob);
-            Runtime.Py_CLEAR(ref self.repr);
-            return 0;
+            Runtime.Py_CLEAR(ref this.repr);
+            base.Clear();
         }
 
         public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg)
@@ -254,11 +253,10 @@ protected override void Dealloc()
             base.Dealloc();
         }
 
-        public static int tp_clear(IntPtr ob)
+        protected override void Clear()
         {
-            var self = (BoundContructor)GetManagedObject(ob);
-            Runtime.Py_CLEAR(ref self.repr);
-            return 0;
+            Runtime.Py_CLEAR(ref this.repr);
+            base.Clear();
         }
 
         public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg)
diff --git a/src/runtime/eventbinding.cs b/src/runtime/eventbinding.cs
index 60b9bba92..8fb0ce250 100644
--- a/src/runtime/eventbinding.cs
+++ b/src/runtime/eventbinding.cs
@@ -109,11 +109,10 @@ protected override void Dealloc()
             base.Dealloc();
         }
 
-        public static int tp_clear(IntPtr ob)
+        protected override void Clear()
         {
-            var self = (EventBinding)GetManagedObject(ob);
-            Runtime.Py_CLEAR(ref self.target);
-            return 0;
+            Runtime.Py_CLEAR(ref this.target);
+            base.Clear();
         }
     }
 }
diff --git a/src/runtime/eventobject.cs b/src/runtime/eventobject.cs
index e9bd98821..a0f06c375 100644
--- a/src/runtime/eventobject.cs
+++ b/src/runtime/eventobject.cs
@@ -207,6 +207,16 @@ protected override void Dealloc()
             }
             base.Dealloc();
         }
+
+        protected override void Clear()
+        {
+            if (this.unbound is not null)
+            {
+                Runtime.XDecref(this.unbound.pyHandle);
+                this.unbound = null;
+            }
+            base.Clear();
+        }
     }
 
 
diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs
index db9eb0f72..38fe238de 100644
--- a/src/runtime/extensiontype.cs
+++ b/src/runtime/extensiontype.cs
@@ -62,6 +62,9 @@ protected virtual void Dealloc()
             this.FreeGCHandle();
         }
 
+        /// <summary>DecRefs and nulls any fields pointing back to Python</summary>
+        protected virtual void Clear() { }
+
         /// <summary>
         /// Type __setattr__ implementation.
         /// </summary>
@@ -88,9 +91,6 @@ public static int tp_descr_set(IntPtr ds, IntPtr ob, IntPtr val)
         }
 
 
-        /// <summary>
-        /// Default dealloc implementation.
-        /// </summary>
         public static void tp_dealloc(IntPtr ob)
         {
             // Clean up a Python instance of this extension type. This
@@ -99,6 +99,13 @@ public static void tp_dealloc(IntPtr ob)
             self?.Dealloc();
         }
 
+        public static int tp_clear(IntPtr ob)
+        {
+            var self = (ExtensionType)GetManagedObject(ob);
+            self?.Clear();
+            return 0;
+        }
+
         protected override void OnLoad(InterDomainContext context)
         {
             base.OnLoad(context);
diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs
index 783717189..a842dd308 100644
--- a/src/runtime/methodbinding.cs
+++ b/src/runtime/methodbinding.cs
@@ -241,11 +241,10 @@ protected override void Dealloc()
             base.Dealloc();
         }
 
-        public static int tp_clear(IntPtr ob)
+        protected override void Clear()
         {
-            var self = (MethodBinding)GetManagedObject(ob);
-            self.ClearMembers();
-            return 0;
+            this.ClearMembers();
+            base.Clear();
         }
 
         protected override void OnSave(InterDomainContext context)
diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs
index 5fa965f1b..c181c5df1 100644
--- a/src/runtime/methodobject.cs
+++ b/src/runtime/methodobject.cs
@@ -217,12 +217,11 @@ protected override void Dealloc()
             base.Dealloc();
         }
 
-        public static int tp_clear(IntPtr ob)
+        protected override void Clear()
         {
-            var self = (MethodObject)GetManagedObject(ob);
-            self.ClearMembers();
-            ClearObjectDict(ob);
-            return 0;
+            this.ClearMembers();
+            ClearObjectDict(this.pyHandle);
+            base.Clear();
         }
 
         protected override void OnSave(InterDomainContext context)
diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs
index 1ab014c2a..b2721cc47 100644
--- a/src/runtime/moduleobject.cs
+++ b/src/runtime/moduleobject.cs
@@ -295,12 +295,6 @@ public static IntPtr tp_repr(IntPtr ob)
             return Runtime.PyString_FromString($"<module '{self.moduleName}'>");
         }
 
-        protected override void Dealloc()
-        {
-            tp_clear(this.pyHandle);
-            base.Dealloc();
-        }
-
         public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg)
         {
             var self = (ModuleObject)GetManagedObject(ob);
@@ -314,17 +308,22 @@ public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg)
             return 0;
         }
 
-        public static int tp_clear(IntPtr ob)
+        protected override void Dealloc()
         {
-            var self = (ModuleObject)GetManagedObject(ob);
-            Runtime.Py_CLEAR(ref self.dict);
-            ClearObjectDict(ob);
-            foreach (var attr in self.cache.Values)
+            tp_clear(this.pyHandle);
+            base.Dealloc();
+        }
+
+        protected override void Clear()
+        {
+            Runtime.Py_CLEAR(ref this.dict);
+            ClearObjectDict(this.pyHandle);
+            foreach (var attr in this.cache.Values)
             {
                 Runtime.XDecref(attr.pyHandle);
             }
-            self.cache.Clear();
-            return 0;
+            this.cache.Clear();
+            base.Clear();
         }
 
         protected override void OnSave(InterDomainContext context)
diff --git a/src/runtime/overload.cs b/src/runtime/overload.cs
index 48fabca4a..c6c3158fb 100644
--- a/src/runtime/overload.cs
+++ b/src/runtime/overload.cs
@@ -63,5 +63,11 @@ protected override void Dealloc()
             Runtime.Py_CLEAR(ref this.target);
             base.Dealloc();
         }
+
+        protected override void Clear()
+        {
+            Runtime.Py_CLEAR(ref this.target);
+            base.Clear();
+        }
     }
 }

From b1dbe5e927cfb5c9e2285e7a2888f4d5cea3457d Mon Sep 17 00:00:00 2001
From: Victor Nova <lost@losttech.software>
Date: Sat, 22 May 2021 23:55:52 -0700
Subject: [PATCH 2/2] all Dealloc overrides simply duplicate Clear, so just
 call both from tp_dealloc and don't override Dealloc

---
 src/runtime/constructorbinding.cs | 12 ------------
 src/runtime/eventbinding.cs       |  6 ------
 src/runtime/eventobject.cs        | 10 ----------
 src/runtime/extensiontype.cs      | 15 ++++++++++++---
 src/runtime/methodbinding.cs      | 15 ++-------------
 src/runtime/methodobject.cs       | 21 +++++----------------
 src/runtime/moduleobject.cs       |  6 ------
 src/runtime/overload.cs           |  6 ------
 8 files changed, 19 insertions(+), 72 deletions(-)

diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs
index da35aafd1..9ac1adc0f 100644
--- a/src/runtime/constructorbinding.cs
+++ b/src/runtime/constructorbinding.cs
@@ -149,12 +149,6 @@ public static IntPtr tp_repr(IntPtr ob)
             return self.repr;
         }
 
-        protected override void Dealloc()
-        {
-            Runtime.Py_CLEAR(ref this.repr);
-            base.Dealloc();
-        }
-
         protected override void Clear()
         {
             Runtime.Py_CLEAR(ref this.repr);
@@ -247,12 +241,6 @@ public static IntPtr tp_repr(IntPtr ob)
             return self.repr;
         }
 
-        protected override void Dealloc()
-        {
-            Runtime.Py_CLEAR(ref this.repr);
-            base.Dealloc();
-        }
-
         protected override void Clear()
         {
             Runtime.Py_CLEAR(ref this.repr);
diff --git a/src/runtime/eventbinding.cs b/src/runtime/eventbinding.cs
index 8fb0ce250..65c8fdccf 100644
--- a/src/runtime/eventbinding.cs
+++ b/src/runtime/eventbinding.cs
@@ -103,12 +103,6 @@ public static IntPtr tp_repr(IntPtr ob)
             return Runtime.PyString_FromString(s);
         }
 
-        protected override void Dealloc()
-        {
-            Runtime.Py_CLEAR(ref this.target);
-            base.Dealloc();
-        }
-
         protected override void Clear()
         {
             Runtime.Py_CLEAR(ref this.target);
diff --git a/src/runtime/eventobject.cs b/src/runtime/eventobject.cs
index a0f06c375..941bbdf46 100644
--- a/src/runtime/eventobject.cs
+++ b/src/runtime/eventobject.cs
@@ -198,16 +198,6 @@ public static IntPtr tp_repr(IntPtr ob)
         }
 
 
-        protected override void Dealloc()
-        {
-            if (this.unbound is not null)
-            {
-                Runtime.XDecref(this.unbound.pyHandle);
-                this.unbound = null;
-            }
-            base.Dealloc();
-        }
-
         protected override void Clear()
         {
             if (this.unbound is not null)
diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs
index 38fe238de..78df805ee 100644
--- a/src/runtime/extensiontype.cs
+++ b/src/runtime/extensiontype.cs
@@ -56,14 +56,22 @@ void SetupGc ()
 
         protected virtual void Dealloc()
         {
-            ClearObjectDict(this.pyHandle);
+            var type = Runtime.PyObject_TYPE(this.ObjectReference);
             Runtime.PyObject_GC_Del(this.pyHandle);
-            // Not necessary for decref of `tpHandle`.
+            // Not necessary for decref of `tpHandle` - it is borrowed
+
             this.FreeGCHandle();
+
+            // we must decref our type: https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_dealloc
+            Runtime.XDecref(type.DangerousGetAddress());
         }
 
         /// <summary>DecRefs and nulls any fields pointing back to Python</summary>
-        protected virtual void Clear() { }
+        protected virtual void Clear()
+        {
+            ClearObjectDict(this.pyHandle);
+            // Not necessary for decref of `tpHandle` - it is borrowed
+        }
 
         /// <summary>
         /// Type __setattr__ implementation.
@@ -96,6 +104,7 @@ public static void tp_dealloc(IntPtr ob)
             // Clean up a Python instance of this extension type. This
             // frees the allocated Python object and decrefs the type.
             var self = (ExtensionType)GetManagedObject(ob);
+            self?.Clear();
             self?.Dealloc();
         }
 
diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs
index a842dd308..c1e729f9e 100644
--- a/src/runtime/methodbinding.cs
+++ b/src/runtime/methodbinding.cs
@@ -229,21 +229,10 @@ public static IntPtr tp_repr(IntPtr ob)
             return Runtime.PyString_FromString($"<{type} method '{name}'>");
         }
 
-        private void ClearMembers()
-        {
-            Runtime.Py_CLEAR(ref target);
-            Runtime.Py_CLEAR(ref targetType);
-        }
-
-        protected override void Dealloc()
-        {
-            this.ClearMembers();
-            base.Dealloc();
-        }
-
         protected override void Clear()
         {
-            this.ClearMembers();
+            Runtime.Py_CLEAR(ref this.target);
+            Runtime.Py_CLEAR(ref this.targetType);
             base.Clear();
         }
 
diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs
index c181c5df1..2787ec999 100644
--- a/src/runtime/methodobject.cs
+++ b/src/runtime/methodobject.cs
@@ -200,26 +200,15 @@ public static IntPtr tp_repr(IntPtr ob)
             return Runtime.PyString_FromString($"<method '{self.name}'>");
         }
 
-        private void ClearMembers()
+        protected override void Clear()
         {
-            Runtime.Py_CLEAR(ref doc);
-            if (unbound != null)
+            Runtime.Py_CLEAR(ref this.doc);
+            if (this.unbound != null)
             {
-                Runtime.XDecref(unbound.pyHandle);
-                unbound = null;
+                Runtime.XDecref(this.unbound.pyHandle);
+                this.unbound = null;
             }
-        }
 
-        protected override void Dealloc()
-        {
-            this.ClearMembers();
-            ClearObjectDict(this.pyHandle);
-            base.Dealloc();
-        }
-
-        protected override void Clear()
-        {
-            this.ClearMembers();
             ClearObjectDict(this.pyHandle);
             base.Clear();
         }
diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs
index b2721cc47..606fa7be9 100644
--- a/src/runtime/moduleobject.cs
+++ b/src/runtime/moduleobject.cs
@@ -308,12 +308,6 @@ public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg)
             return 0;
         }
 
-        protected override void Dealloc()
-        {
-            tp_clear(this.pyHandle);
-            base.Dealloc();
-        }
-
         protected override void Clear()
         {
             Runtime.Py_CLEAR(ref this.dict);
diff --git a/src/runtime/overload.cs b/src/runtime/overload.cs
index c6c3158fb..8222dc136 100644
--- a/src/runtime/overload.cs
+++ b/src/runtime/overload.cs
@@ -58,12 +58,6 @@ public static IntPtr tp_repr(IntPtr op)
             return doc;
         }
 
-        protected override void Dealloc()
-        {
-            Runtime.Py_CLEAR(ref this.target);
-            base.Dealloc();
-        }
-
         protected override void Clear()
         {
             Runtime.Py_CLEAR(ref this.target);