Fix Shutdown crash

amos402 · filmor · commit 6ebf48512114 · 2021-02-22T10:32:11.000+01:00
diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs
@@ -354,9 +354,10 @@ public static IntPtr tp_repr(IntPtr ob)
         public static void tp_dealloc(IntPtr ob)
         {
             ManagedType self = GetManagedObject(ob);
-            tp_clear(ob);
-            Runtime.PyObject_GC_UnTrack(self.pyHandle);
-            Runtime.PyObject_GC_Del(self.pyHandle);
+            RemoveObjectDict(ob);
+            Runtime.Py_CLEAR(ref self.tpHandle);
+            Runtime.PyObject_GC_UnTrack(ob);
+            Runtime.PyObject_GC_Del(ob);
             self.FreeGCHandle();
         }
 
diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs
@@ -5,6 +5,7 @@
 using System.Runtime.InteropServices;
 using System.Security;
 using System.Linq;
+using System.Diagnostics;
 
 namespace Python.Runtime
 {
@@ -270,7 +271,7 @@ private static void InitClassBase(Type type, ClassBase impl)
 
             // Finally, initialize the class __dict__ and return the object.
             var dict = new BorrowedReference(Marshal.ReadIntPtr(tp, TypeOffset.tp_dict));
-
+            Debug.Assert(!dict.IsNull);
 
             if (impl.dotNetMembers == null)
             {
diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs
@@ -55,7 +55,7 @@ void SetupGc ()
         /// </summary>
         public static void FinalizeObject(ManagedType self)
         {
-            ClearObjectDict(self.pyHandle);
+            RemoveObjectDict(self.pyHandle);
             Runtime.PyObject_GC_Del(self.pyHandle);
             // Not necessary for decref of `tpHandle`.
             self.FreeGCHandle();
diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs
@@ -243,6 +243,16 @@ protected virtual void OnSave(InterDomainContext context) { }
         protected virtual void OnLoad(InterDomainContext context) { }
 
         protected static void ClearObjectDict(IntPtr ob)
+        {
+            IntPtr dict = GetObjectDict(ob);
+            if (dict == IntPtr.Zero)
+            {
+                return;
+            }
+            Runtime.PyDict_Clear(dict);
+        }
+
+        protected static void RemoveObjectDict(IntPtr ob)
         {
             IntPtr dict = GetObjectDict(ob);
             if (dict == IntPtr.Zero)
diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs
@@ -304,6 +304,12 @@ public static void tp_dealloc(IntPtr tp)
             NativeCall.Void_Call_1(op, tp);
         }
 
+        public static int tp_clear(IntPtr ob)
+        {
+            ClearObjectDict(ob);
+            return 0;
+        }
+
         private static IntPtr DoInstanceCheck(IntPtr tp, IntPtr args, bool checkType)
         {
             var cb = GetManagedObject(tp) as ClassBase;
diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs
@@ -217,7 +217,7 @@ public static IntPtr tp_repr(IntPtr ob)
         {
             var self = (MethodObject)GetManagedObject(ob);
             self.ClearMembers();
-            ClearObjectDict(ob);
+            RemoveObjectDict(ob);
             self.Dealloc();
         }
 
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -503,15 +503,19 @@ private static void PyDictTryDelItem(BorrowedReference dict, string key)
         private static void MoveClrInstancesOnwershipToPython()
         {
             var objs = ManagedType.GetManagedObjects();
-            var copyObjs = objs.ToArray();
-            foreach (var entry in copyObjs)
+            var copyObjs = new KeyValuePair<ManagedType, ManagedType.TrackTypes>[objs.Count];
             {
-                ManagedType obj = entry.Key;
-                if (!objs.ContainsKey(obj))
+                int i = 0;
+                foreach (var entry in objs)
                 {
-                    System.Diagnostics.Debug.Assert(obj.gcHandle == default);
-                    continue;
+                    ManagedType obj = entry.Key;
+                    XIncref(obj.pyHandle);
+                    copyObjs[i++] = entry;
                 }
+            }
+            foreach (var entry in copyObjs)
+            {
+                ManagedType obj = entry.Key;
                 if (entry.Value == ManagedType.TrackTypes.Extension)
                 {
                     obj.CallTypeClear();
@@ -522,11 +526,21 @@ private static void MoveClrInstancesOnwershipToPython()
                         PyObject_GC_Track(obj.pyHandle);
                     }
                 }
-                if (obj.gcHandle.IsAllocated)
+            }
+            foreach (var entry in copyObjs)
+            {
+                ManagedType obj = entry.Key;
+                if (!objs.ContainsKey(obj))
+                {
+                    System.Diagnostics.Debug.Assert(obj.gcHandle == default);
+                    continue;
+                }
+                if (obj.RefCount > 1)
                 {
-                    obj.gcHandle.Free();
+                    obj.FreeGCHandle();
+                    Marshal.WriteIntPtr(obj.pyHandle, ObjectOffset.magic(obj.tpHandle), IntPtr.Zero);
                 }
-                obj.gcHandle = default;
+                XDecref(obj.pyHandle);
             }
             ManagedType.ClearTrackedObjects();
         }

Original file line number	Diff line number	Diff line change
`@@ -354,9 +354,10 @@ public static IntPtr tp_repr(IntPtr ob)`
`354`	`354`	`public static void tp_dealloc(IntPtr ob)`
`355`	`355`	`{`
`356`	`356`	`ManagedType self = GetManagedObject(ob);`
`357`		`- tp_clear(ob);`
`358`		`- Runtime.PyObject_GC_UnTrack(self.pyHandle);`
`359`		`- Runtime.PyObject_GC_Del(self.pyHandle);`
	`357`	`+ RemoveObjectDict(ob);`
	`358`	`+ Runtime.Py_CLEAR(ref self.tpHandle);`
	`359`	`+ Runtime.PyObject_GC_UnTrack(ob);`
	`360`	`+ Runtime.PyObject_GC_Del(ob);`
`360`	`361`	`self.FreeGCHandle();`
`361`	`362`	`}`
`362`	`363`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`	`using System.Runtime.InteropServices;`
`6`	`6`	`using System.Security;`
`7`	`7`	`using System.Linq;`
	`8`	`+using System.Diagnostics;`
`8`	`9`
`9`	`10`	`namespace Python.Runtime`
`10`	`11`	`{`
`@@ -270,7 +271,7 @@ private static void InitClassBase(Type type, ClassBase impl)`
`270`	`271`
`271`	`272`	`// Finally, initialize the class __dict__ and return the object.`
`272`	`273`	`var dict = new BorrowedReference(Marshal.ReadIntPtr(tp, TypeOffset.tp_dict));`
`273`		`-`
	`274`	`+ Debug.Assert(!dict.IsNull);`
`274`	`275`
`275`	`276`	`if (impl.dotNetMembers == null)`
`276`	`277`	`{`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ void SetupGc ()`
`55`	`55`	`/// </summary>`
`56`	`56`	`public static void FinalizeObject(ManagedType self)`
`57`	`57`	`{`
`58`		`- ClearObjectDict(self.pyHandle);`
	`58`	`+ RemoveObjectDict(self.pyHandle);`
`59`	`59`	`Runtime.PyObject_GC_Del(self.pyHandle);`
`60`	`60`	// Not necessary for decref of `tpHandle`.
`61`	`61`	`self.FreeGCHandle();`
Original file line number	Diff line number	Diff line change
`@@ -304,6 +304,12 @@ public static void tp_dealloc(IntPtr tp)`
`304`	`304`	`NativeCall.Void_Call_1(op, tp);`
`305`	`305`	`}`
`306`	`306`
	`307`	`+ public static int tp_clear(IntPtr ob)`
	`308`	`+ {`
	`309`	`+ ClearObjectDict(ob);`
	`310`	`+ return 0;`
	`311`	`+ }`
	`312`	`+`
`307`	`313`	`private static IntPtr DoInstanceCheck(IntPtr tp, IntPtr args, bool checkType)`
`308`	`314`	`{`
`309`	`315`	`var cb = GetManagedObject(tp) as ClassBase;`
Original file line number	Diff line number	Diff line change
`@@ -217,7 +217,7 @@ public static IntPtr tp_repr(IntPtr ob)`
`217`	`217`	`{`
`218`	`218`	`var self = (MethodObject)GetManagedObject(ob);`
`219`	`219`	`self.ClearMembers();`
`220`		`- ClearObjectDict(ob);`
	`220`	`+ RemoveObjectDict(ob);`
`221`	`221`	`self.Dealloc();`
`222`	`222`	`}`
`223`	`223`
Original file line number	Diff line number	Diff line change
`@@ -503,15 +503,19 @@ private static void PyDictTryDelItem(BorrowedReference dict, string key)`
`503`	`503`	`private static void MoveClrInstancesOnwershipToPython()`
`504`	`504`	`{`
`505`	`505`	`var objs = ManagedType.GetManagedObjects();`
`506`		`- var copyObjs = objs.ToArray();`
`507`		`- foreach (var entry in copyObjs)`
	`506`	`+ var copyObjs = new KeyValuePair<ManagedType, ManagedType.TrackTypes>[objs.Count];`
`508`	`507`	`{`
`509`		`- ManagedType obj = entry.Key;`
`510`		`- if (!objs.ContainsKey(obj))`
	`508`	`+ int i = 0;`
	`509`	`+ foreach (var entry in objs)`
`511`	`510`	`{`
`512`		`- System.Diagnostics.Debug.Assert(obj.gcHandle == default);`
`513`		`- continue;`
	`511`	`+ ManagedType obj = entry.Key;`
	`512`	`+ XIncref(obj.pyHandle);`
	`513`	`+ copyObjs[i++] = entry;`
`514`	`514`	`}`
	`515`	`+ }`
	`516`	`+ foreach (var entry in copyObjs)`
	`517`	`+ {`
	`518`	`+ ManagedType obj = entry.Key;`
`515`	`519`	`if (entry.Value == ManagedType.TrackTypes.Extension)`
`516`	`520`	`{`
`517`	`521`	`obj.CallTypeClear();`
`@@ -522,11 +526,21 @@ private static void MoveClrInstancesOnwershipToPython()`
`522`	`526`	`PyObject_GC_Track(obj.pyHandle);`
`523`	`527`	`}`
`524`	`528`	`}`
`525`		`- if (obj.gcHandle.IsAllocated)`
	`529`	`+ }`
	`530`	`+ foreach (var entry in copyObjs)`
	`531`	`+ {`
	`532`	`+ ManagedType obj = entry.Key;`
	`533`	`+ if (!objs.ContainsKey(obj))`
	`534`	`+ {`
	`535`	`+ System.Diagnostics.Debug.Assert(obj.gcHandle == default);`
	`536`	`+ continue;`
	`537`	`+ }`
	`538`	`+ if (obj.RefCount > 1)`
`526`	`539`	`{`
`527`		`- obj.gcHandle.Free();`
	`540`	`+ obj.FreeGCHandle();`
	`541`	`+ Marshal.WriteIntPtr(obj.pyHandle, ObjectOffset.magic(obj.tpHandle), IntPtr.Zero);`
`528`	`542`	`}`
`529`		`- obj.gcHandle = default;`
	`543`	`+ XDecref(obj.pyHandle);`
`530`	`544`	`}`
`531`	`545`	`ManagedType.ClearTrackedObjects();`
`532`	`546`	`}`