disallow runtime shutdown when the Python error indicator is set, as it may lead to unpredictable behavior (#1780)

lostmsu · web-flow · commit a80c685d197d · 2022-05-04T20:11:13.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,6 +36,7 @@ and other `PyObject` derived types when called from Python.
 details about the cause of the failure
 -   `clr.AddReference` no longer adds ".dll" implicitly
 -   `PyIter(PyObject)` constructor replaced with static `PyIter.GetIter(PyObject)` method
+-   Python runtime can no longer be shut down if the Python error indicator is set, as it would have unpredictable behavior
 -   BREAKING: Return values from .NET methods that return an interface are now automatically
      wrapped in that interface. This is a breaking change for users that rely on being
      able to access members that are part of the implementation class, but not the
diff --git a/src/runtime/PythonEngine.cs b/src/runtime/PythonEngine.cs
@@ -351,6 +351,12 @@ public static void Shutdown()
             {
                 return;
             }
+            if (Exceptions.ErrorOccurred())
+            {
+                throw new InvalidOperationException(
+                    "Python error indicator is set",
+                    innerException: PythonException.PeekCurrentOrNull(out _));
+            }
             // If the shutdown handlers trigger a domain unload,
             // don't call shutdown again.
             AppDomain.CurrentDomain.DomainUnload -= OnDomainUnload;
diff --git a/src/runtime/PythonException.cs b/src/runtime/PythonException.cs
@@ -75,6 +75,23 @@ internal static PythonException FetchCurrentRaw()
             => FetchCurrentOrNullRaw()
                ?? throw new InvalidOperationException("No exception is set");
 
+        internal static Exception? PeekCurrentOrNull(out ExceptionDispatchInfo? dispatchInfo)
+        {
+            using var _ = new Py.GILState();
+
+            Runtime.PyErr_Fetch(out var type, out var value, out var traceback);
+            Runtime.PyErr_Restore(
+                new NewReference(type, canBeNull: true).StealNullable(),
+                new NewReference(value, canBeNull: true).StealNullable(),
+                new NewReference(traceback, canBeNull: true).StealNullable());
+
+            var err = FetchCurrentOrNull(out dispatchInfo);
+
+            Runtime.PyErr_Restore(type.StealNullable(), value.StealNullable(), traceback.StealNullable());
+
+            return err;
+        }
+
         internal static Exception? FetchCurrentOrNull(out ExceptionDispatchInfo? dispatchInfo)
         {
             dispatchInfo = null;

Original file line number	Diff line number	Diff line change
`@@ -351,6 +351,12 @@ public static void Shutdown()`
`351`	`351`	`{`
`352`	`352`	`return;`
`353`	`353`	`}`
	`354`	`+ if (Exceptions.ErrorOccurred())`
	`355`	`+ {`
	`356`	`+ throw new InvalidOperationException(`
	`357`	`+ "Python error indicator is set",`
	`358`	`+ innerException: PythonException.PeekCurrentOrNull(out _));`
	`359`	`+ }`
`354`	`360`	`// If the shutdown handlers trigger a domain unload,`
`355`	`361`	`// don't call shutdown again.`
`356`	`362`	`AppDomain.CurrentDomain.DomainUnload -= OnDomainUnload;`