Retry GIL acquisition on exception

Author: msimacek

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextBuiltins.java

@@ -290,7 +290,7 @@ public static PException checkThrowableBeforeNative(Throwable t, String where1,
         if (t instanceof StackOverflowError soe) {
             CompilerDirectives.transferToInterpreter();
             PythonContext context = PythonContext.get(null);
-            context.reacquireGilAfterStackOverflow();
+            context.ensureGilAfterFailure();
             PBaseException newException = context.factory().createBaseException(RecursionError, ErrorMessages.MAXIMUM_RECURSION_DEPTH_EXCEEDED, EMPTY_OBJECT_ARRAY);
             throw ExceptionUtils.wrapJavaException(soe, null, newException);
         }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/hpy/GraalHPyNativeContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -221,7 +221,7 @@ public static PException checkThrowableBeforeNative(Throwable t, String where1,
         if (t instanceof StackOverflowError soe) {
             CompilerDirectives.transferToInterpreter();
             PythonContext context = PythonContext.get(null);
-            context.reacquireGilAfterStackOverflow();
+            context.ensureGilAfterFailure();
             PBaseException newException = context.factory().createBaseException(RecursionError, ErrorMessages.MAXIMUM_RECURSION_DEPTH_EXCEEDED, EMPTY_OBJECT_ARRAY);
             throw ExceptionUtils.wrapJavaException(soe, null, newException);
         }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/GilNode.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -88,7 +88,18 @@ public void release(PythonContext context, boolean wasAcquired) {
         @Override
         public boolean acquire(PythonContext context, Node location) {
             if (binaryProfile.profile(!context.ownsGil())) {
-                TruffleSafepoint.setBlockedThreadInterruptible(location, PythonContext::acquireGil, context);
+                try {
+                    TruffleSafepoint.setBlockedThreadInterruptible(location, PythonContext::acquireGil, context);
+                } catch (Throwable t) {
+                    /*
+                     * Safepoint actions may throw exceptions, so we need to make sure that we
+                     * really acquire the GIL in the end before we hand the exception to python. And
+                     * let's not allow safepoint actions anymore so the exception doesn't get
+                     * swallowed.
+                     */
+                    context.ensureGilAfterFailure();
+                    throw t;
+                }
                 return true;
             }
             return false;
@@ -128,7 +139,18 @@ public final void release(boolean wasAcquired) {
         public final boolean acquire(PythonContext context, Node location) {
             if (!context.ownsGil()) {
                 if (!context.tryAcquireGil()) {
-                    TruffleSafepoint.setBlockedThreadInterruptible(location, PythonContext::acquireGil, context);
+                    try {
+                        TruffleSafepoint.setBlockedThreadInterruptible(location, PythonContext::acquireGil, context);
+                    } catch (Throwable t) {
+                        /*
+                         * Safepoint actions may throw exceptions, so we need to make sure that we
+                         * really acquire the GIL in the end before we hand the exception to python.
+                         * And let's not allow safepoint actions anymore so the exception doesn't
+                         * get swallowed.
+                         */
+                        context.ensureGilAfterFailure();
+                        throw t;
+                    }
                 }
                 return true;
             }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java

@@ -2234,18 +2234,16 @@ public CyclicAssumption getNativeClassStableAssumption(PythonNativeClass cls, bo
     }
 
     /**
-     * This method is intended to be called to re-acquire the GIL after a {@link StackOverflowError}
-     * was catched. To reduce the probability that re-acquiring the GIL causes again a
-     * {@link StackOverflowError}, it is important to keep this method as simple as possible. In
-     * particular, do not add calls if there is a way to avoid it.
+     * Acquire GIL after the normal GIL acquisition failed or there is a possibility that finally
+     * blocks were skipped, such as after a {@link StackOverflowError}. To reduce the probability
+     * that re-acquiring the GIL causes again a {@link StackOverflowError}, it is important to keep
+     * this method as simple as possible. In particular, do not add calls if there is a way to avoid
+     * it.
      */
-    public void reacquireGilAfterStackOverflow() {
-        while (!ownsGil()) {
-            try {
-                acquireGil();
-            } catch (InterruptedException ignored) {
-                // just keep trying
-            }
+    @TruffleBoundary
+    public void ensureGilAfterFailure() {
+        if (!ownsGil()) {
+            globalInterpreterLock.lock();
         }
     }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/exception/ExceptionUtils.java

@@ -273,7 +273,7 @@ public static PException wrapJavaException(Throwable e, Node node, PBaseExceptio
     public static PException wrapJavaExceptionIfApplicable(Node location, Throwable e, PythonObjectFactory factory) {
         if (e instanceof StackOverflowError) {
             CompilerDirectives.transferToInterpreterAndInvalidate();
-            PythonContext.get(null).reacquireGilAfterStackOverflow();
+            PythonContext.get(null).ensureGilAfterFailure();
             return ExceptionUtils.wrapJavaException(e, location, factory.createBaseException(RecursionError, ErrorMessages.MAXIMUM_RECURSION_DEPTH_EXCEEDED, new Object[]{}));
         }
         if (e instanceof OutOfMemoryError) {