Ensure GIL is released after exception is thrown (#27013)

Ensure that the Python GIL is released when an exception is thrown from
an initializer.

The pattern taken by our throwing initializers for the Python module is
that the super class Value.init acquires the GIL, and then the super
class Value.postinit releases the GIL (after all init's have fired).
However, based on recent discussions (see
#26997 and
#26990) these calls cannot
rely on a postinit being called when an exception is thrown.

Prior to this PR, if an exception was thrown in the init, the GIL would
never be released. This PR resolves the problem by explicitly handling
the exception in the init, releasing the GIL, and then rethrowing the
caught exception.

- [x] Ran `start_test test/library/packages/Python`

[Reviewed by @lydia-duncan]

Author: jabraham17

modules/packages/Python.chpl

@@ -1834,7 +1834,19 @@ module Python {
       this.isOwned = true;
       this.ctxInitState = chpl_pythonContext.enter();
       init this;
-      this.obj = this.interpreter.loadPickle(pickleData);
+      // Only catch-less try! statements are allowed in initializers for now :(
+      proc helper() throws {
+        try {
+          this.obj = this.interpreter.loadPickle(pickleData);
+        } catch e {
+          // an exception thrown from the init will not result in a call to the postinit
+          // or the deinit, so we have to handle that stuff here
+          defer PyGILState_Release(this.gilInitState);
+          // rethrow the exception
+          throw e;
+        }
+      }
+      helper();
     }
     /*
       Creates a new Python object from a Chapel value.
@@ -1847,7 +1859,19 @@ module Python {
       this.isOwned = true;
       this.ctxInitState = chpl_pythonContext.enter();
       init this;
-      this.obj = this.interpreter.toPythonInner(value);
+      // Only catch-less try! statements are allowed in initializers for now :(
+      proc helper() throws {
+        try {
+          this.obj = this.interpreter.toPythonInner(value);
+        } catch e {
+          // an exception thrown from the init will not result in a call to the postinit
+          // or the deinit, so we have to handle that stuff here
+          defer PyGILState_Release(this.gilInitState);
+          // rethrow the exception
+          throw e;
+        }
+      }
+      helper();
     }
 
     @chpldoc.nodoc
@@ -2415,7 +2439,19 @@ module Python {
       super.init(interpreter, nil: PyObjectPtr, isOwned=true);
       this.modName = modName;
       init this;
-      this.obj = interpreter.compileModule(moduleContents, modName);
+      // Only catch-less try! statements are allowed in initializers for now :(
+      proc helper() throws {
+        try {
+          this.obj = interpreter.compileModule(moduleContents, modName);
+        } catch e {
+          // an exception thrown from the init will not result in a call to the postinit
+          // or the deinit, so we have to handle that stuff here
+          defer PyGILState_Release(this.gilInitState);
+          // rethrow the exception
+          throw e;
+        }
+      }
+      helper();
     }
 
     /*
@@ -2427,7 +2463,19 @@ module Python {
       super.init(interpreter, nil: PyObjectPtr, isOwned=true);
       this.modName = modName;
       init this;
-      this.obj = interpreter.compileModule(moduleBytecode, modName);
+      // Only catch-less try! statements are allowed in initializers for now :(
+      proc helper() throws {
+        try {
+          this.obj = interpreter.compileModule(moduleBytecode, modName);
+        } catch e {
+          // an exception thrown from the init will not result in a call to the postinit
+          // or the deinit, so we have to handle that stuff here
+          defer PyGILState_Release(this.gilInitState);
+          // rethrow the exception
+          throw e;
+        }
+      }
+      helper();
     }
   }
 
@@ -2482,7 +2530,19 @@ module Python {
       super.init(interpreter, nil: PyObjectPtr, isOwned=true);
       this.fnName = "<anon>";
       init this;
-      this.obj = interpreter.compileLambdaInternal(lambdaFn);
+      // Only catch-less try! statements are allowed in initializers for now :(
+      proc helper() throws {
+        try {
+          this.obj = interpreter.compileLambdaInternal(lambdaFn);
+        } catch e {
+          // an exception thrown from the init will not result in a call to the postinit
+          // or the deinit, so we have to handle that stuff here
+          defer PyGILState_Release(this.gilInitState);
+          // rethrow the exception
+          throw e;
+        }
+      }
+      helper();
     }
     @chpldoc.nodoc
     proc init(in interpreter: borrowed Interpreter,
@@ -3403,15 +3463,15 @@ module Python {
       :arg interpreter: The interpreter that this object is associated with.
       :arg arr: The Chapel array to create the object from.
     */
-    proc init(in interpreter: borrowed Interpreter, ref arr: []) throws
+    proc init(in interpreter: borrowed Interpreter, ref arr: [])
       where isSupportedArrayType(arr) {
       super.init(interpreter, nil: PyObjectPtr, isOwned=true);
       this.eltType = arr.eltType;
       init this;
       this.obj = ArrayTypes.createArray(arr);
     }
     @chpldoc.nodoc
-    proc init(in interpreter: borrowed Interpreter, ref arr: []) throws
+    proc init(in interpreter: borrowed Interpreter, ref arr: [])
       where !isSupportedArrayType(arr) {
       super.init(interpreter, nil: PyObjectPtr, isOwned=false);
       compilerError("Only 1D local rectangular arrays are currently supported");

test/library/packages/Python/correctness/exceptionsFromInit.chpl

@@ -0,0 +1,35 @@
+//
+// Test case of a variety of possible scenarios where an exception could be thrown
+// This test makes sure the exception is caught and the program does not hang
+// test/library/packages/Python/correctness/unsupportedTypes.chpl tests something similar
+//
+
+
+use Python;
+var i = new Interpreter();
+
+try {
+  var f = new Function(i, "lambda x,:");
+} catch e {
+  writeln("Caught exception: ", e);
+}
+
+try {
+  var fakePklData: bytes = b"fakePklData";
+  var v = new Value(i, fakePklData);
+} catch e {
+  writeln("Caught exception: ", e);
+}
+
+try {
+  var fakeBytecode: bytes = b"fakeBytecode";
+  var m = new Module(i, "foo", fakeBytecode);
+} catch e {
+  writeln("Caught exception: ", e);
+}
+
+try {
+  var m = new Module(i, "foo", "I am a syntax error!");
+} catch e {
+  writeln("Caught exception: ", e);
+}

test/library/packages/Python/correctness/exceptionsFromInit.good

@@ -0,0 +1,4 @@
+Caught exception: PythonException: invalid syntax (<string>, line 1)
+Caught exception: PythonException: invalid load key, 'f'.
+Caught exception: PythonException: marshal data too short
+Caught exception: PythonException: invalid syntax (<string>, line 1)