Merge pull request #3 from Unity-Technologies/soft-shutdown-review-comments-2

amos402 · web-flow · commit 12c0206b9417 · 2020-09-11T09:30:53.000+08:00
Soft shutdown review comments 2
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -127,8 +127,8 @@ internal static Version PyVersion
         /// <summary>
         /// Initialize the runtime...
         /// </summary>
-        /// <remarks>When calling this method after a soft shutdown or a domain reload,
-        ///  this method acquires and releases the GIL. </remarks>
+        /// <remarks>Always call this method from the Main thread.  After the 
+        /// first call to this method, the main thread has acquired the GIL.</remarks>
         internal static void Initialize(bool initSigs = false, ShutdownMode mode = ShutdownMode.Default, bool fromPython = false)
         {
             if (_isInitialized)
@@ -407,15 +407,21 @@ internal static void Shutdown(ShutdownMode mode)
                 GC.Collect();
                 try
                 {
-                    GC.WaitForFullGCComplete();
+                    GC.WaitForPendingFinalizers();
                 }
                 catch (NotImplementedException)
                 {
                     // Some clr runtime didn't implement GC.WaitForFullGCComplete yet.
                 }
                 PyGILState_Release(state);
-                // Then release the GIL for good.
-                PyEval_SaveThread();
+                // Then release the GIL for good, if there is somehting to release
+                // Use the unchecked version as the checked version calls `abort()`
+                // if the current state is NULL.
+                if (_PyThreadState_UncheckedGet() != IntPtr.Zero)
+                {
+                    PyEval_SaveThread();
+                }
+                
             }
             else
             {
@@ -846,6 +852,9 @@ internal static unsafe long Refcount(IntPtr op)
         [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
         internal static extern IntPtr PyThreadState_Get();
 
+        [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
+        internal static extern IntPtr _PyThreadState_UncheckedGet();
+
         [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
         internal static extern IntPtr PyThread_get_key_value(IntPtr key);
 
diff --git a/src/runtime/runtime_data.cs b/src/runtime/runtime_data.cs
@@ -137,6 +137,19 @@ public static void ClearStash()
             PySys_SetObject("clr_data", IntPtr.Zero);
         }
 
+        static bool CheckSerializable (object o)
+        {
+            Type type = o.GetType();
+            do
+            {
+                if (!type.IsSerializable)
+                {
+                    return false;
+                }
+            } while ((type = type.BaseType) != null);
+            return true;
+        }
+
         private static void SaveRuntimeDataObjects(RuntimeDataStorage storage)
         {
             var objs = ManagedType.GetManagedObjects();
@@ -151,7 +164,7 @@ private static void SaveRuntimeDataObjects(RuntimeDataStorage storage)
                 switch (entry.Value)
                 {
                     case ManagedType.TrackTypes.Extension:
-                        Debug.Assert(obj.GetType().IsSerializable);
+                        Debug.Assert(CheckSerializable(obj));
                         var context = new InterDomainContext();
                         contexts[obj.pyHandle] = context;
                         obj.Save(context);
@@ -195,7 +208,7 @@ private static void SaveRuntimeDataObjects(RuntimeDataStorage storage)
             {
                 if (!item.Stored)
                 {
-                    if (!item.Instance.GetType().IsSerializable)
+                    if (!CheckSerializable(item.Instance))
                     {
                         continue;
                     }

Original file line number	Diff line number	Diff line change
`@@ -137,6 +137,19 @@ public static void ClearStash()`
`137`	`137`	`PySys_SetObject("clr_data", IntPtr.Zero);`
`138`	`138`	`}`
`139`	`139`
	`140`	`+ static bool CheckSerializable (object o)`
	`141`	`+ {`
	`142`	`+ Type type = o.GetType();`
	`143`	`+ do`
	`144`	`+ {`
	`145`	`+ if (!type.IsSerializable)`
	`146`	`+ {`
	`147`	`+ return false;`
	`148`	`+ }`
	`149`	`+ } while ((type = type.BaseType) != null);`
	`150`	`+ return true;`
	`151`	`+ }`
	`152`	`+`
`140`	`153`	`private static void SaveRuntimeDataObjects(RuntimeDataStorage storage)`
`141`	`154`	`{`
`142`	`155`	`var objs = ManagedType.GetManagedObjects();`
`@@ -151,7 +164,7 @@ private static void SaveRuntimeDataObjects(RuntimeDataStorage storage)`
`151`	`164`	`switch (entry.Value)`
`152`	`165`	`{`
`153`	`166`	`case ManagedType.TrackTypes.Extension:`
`154`		`- Debug.Assert(obj.GetType().IsSerializable);`
	`167`	`+ Debug.Assert(CheckSerializable(obj));`
`155`	`168`	`var context = new InterDomainContext();`
`156`	`169`	`contexts[obj.pyHandle] = context;`
`157`	`170`	`obj.Save(context);`
`@@ -195,7 +208,7 @@ private static void SaveRuntimeDataObjects(RuntimeDataStorage storage)`
`195`	`208`	`{`
`196`	`209`	`if (!item.Stored)`
`197`	`210`	`{`
`198`		`- if (!item.Instance.GetType().IsSerializable)`
	`211`	`+ if (!CheckSerializable(item.Instance))`
`199`	`212`	`{`
`200`	`213`	`continue;`
`201`	`214`	`}`