Delete now useless system threads interrupts in finalizeContext

Author: steve-s

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java

@@ -775,8 +775,6 @@ void runBackgroundGCTask(PythonContext context) {
             return;
         }
         backgroundGCTaskThread = context.createSystemThread(gcTask);
-        backgroundGCTaskThread.setDaemon(true);
-        backgroundGCTaskThread.setName("python-gc-task");
         backgroundGCTaskThread.start();
     }
 
@@ -916,7 +914,8 @@ private void addNativeFinalizer(PythonContext context, Object finalizingPointerO
                 long finalizingPointer = lib.asPointer(finalizingPointerObj);
                 // We are writing off heap memory and registering a VM shutdown hook, there is no
                 // point in creating this thread via Truffle sandbox at this point
-                nativeFinalizerShutdownHook = new Thread(() -> unsafe.putInt(finalizingPointer, 1));
+                nativeFinalizerRunnable = () -> unsafe.putInt(finalizingPointer, 1);
+                nativeFinalizerShutdownHook = new Thread(nativeFinalizerRunnable);
                 Runtime.getRuntime().addShutdownHook(nativeFinalizerShutdownHook);
             } catch (UnsupportedMessageException e) {
                 throw new RuntimeException(e);
@@ -957,14 +956,16 @@ public void exitCApiContext() {
     @SuppressWarnings("try")
     public void finalizeCApi() {
         CompilerAsserts.neverPartOfCompilation();
-        HandleContext handleContext = getContext().nativeContext;
+        PythonContext context = getContext();
+        HandleContext handleContext = context.nativeContext;
         if (backgroundGCTaskThread != null && backgroundGCTaskThread.isAlive()) {
+            context.killSystemThread(backgroundGCTaskThread);
             try {
-                backgroundGCTaskThread.interrupt();
-                backgroundGCTaskThread.join();
+                backgroundGCTaskThread.join(10);
             } catch (InterruptedException e) {
-                Thread.currentThread().interrupt();
+                LOGGER.finest("got interrupt while joining GC thread before cleaning up C API state");
             }
+            backgroundGCTaskThread = null;
         }
 
         /*

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

@@ -421,7 +421,6 @@ public enum LLVMType {
     public final AtomicReference<GraalHPyHandleReference> references = new AtomicReference<>(null);
     private ReferenceQueue<Object> nativeSpaceReferenceQueue;
     @CompilationFinal private RootCallTarget referenceCleanerCallTarget;
-    private Thread hpyReferenceCleanerThread;
 
     private long nativeSpacePointers;
 
@@ -1141,9 +1140,7 @@ private ReferenceQueue<Object> createReferenceQueue() {
         PythonContext context = getContext();
         Env env = context.getEnv();
         if (env.isCreateThreadAllowed()) {
-            Thread thread = context.createSystemThread(new GraalHPyReferenceCleanerRunnable(referenceQueue));
-            thread.start();
-            hpyReferenceCleanerThread = thread;
+            context.createSystemThread(new GraalHPyReferenceCleanerRunnable(referenceQueue)).start();
         } else {
             context.registerAsyncAction(() -> {
                 Reference<?> reference = null;
@@ -1196,17 +1193,6 @@ public Object getNativeNull() {
      * Join the reference cleaner thread.
      */
     public void finalizeContext() {
-        Thread thread = this.hpyReferenceCleanerThread;
-        if (thread != null) {
-            if (thread.isAlive() && !thread.isInterrupted()) {
-                thread.interrupt();
-            }
-            try {
-                thread.join();
-            } catch (InterruptedException e) {
-                // ignore
-            }
-        }
         backend.finalizeNativeContext();
         if (nativeArgumentsStack != 0) {
             UNSAFE.freeMemory(nativeArgumentsStack);

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

@@ -67,8 +67,6 @@ public class NativeBufferContext {
     // Storing references here to keep them alive
     private ConcurrentHashMap<NativePrimitiveReference, NativePrimitiveReference> phantomReferences;
 
-    private Thread nativeBufferReferenceCleanerThread;
-
     @TruffleBoundary
     public void initReferenceQueue() {
         var context = PythonContext.get(null);
@@ -77,9 +75,7 @@ public void initReferenceQueue() {
         TruffleLanguage.Env env = context.getEnv();
         if (env.isCreateThreadAllowed()) {
             var runnable = new NativeBufferDeallocatorRunnable(referenceQueue, this.phantomReferences);
-            Thread thread = context.createSystemThread(runnable);
-            thread.start();
-            this.nativeBufferReferenceCleanerThread = thread;
+            context.createSystemThread(runnable).start();
         }
     }
 
@@ -92,20 +88,6 @@ public NativeIntSequenceStorage createNativeIntStorage(NativeBuffer valueBuffer,
         return storage;
     }
 
-    public void finalizeContext() {
-        Thread thread = this.nativeBufferReferenceCleanerThread;
-        if (thread != null) {
-            if (thread.isAlive() && !thread.isInterrupted()) {
-                thread.interrupt();
-            }
-            try {
-                thread.join();
-            } catch (InterruptedException e) {
-                // ignore
-            }
-        }
-    }
-
     public NativeIntSequenceStorage toNativeIntStorage(int[] arr) {
         long sizeInBytes = (long) arr.length * (long) Integer.BYTES;
         NativeBuffer nativeBuffer = NativeBuffer.allocateNew(sizeInBytes);

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

@@ -2089,7 +2089,6 @@ public void finalizeContext() {
         // interrupt and join or kill system threads
         joinSystemThreads();
         cleanupHPyResources();
-        nativeBufferContext.finalizeContext();
         for (int fd : getChildContextFDs()) {
             if (!getSharedMultiprocessingData().decrementFDRefCount(fd)) {
                 getSharedMultiprocessingData().closePipe(fd);
@@ -2305,13 +2304,7 @@ private void joinSystemThreads() {
             LOGGER.finest("joining thread " + thread);
             int tries = 100;
             for (int i = 0; i < tries && thread.isAlive(); i++) {
-                env.submitThreadLocal(new Thread[]{thread}, new ThreadLocalAction(true, false) {
-                    @Override
-                    protected void perform(ThreadLocalAction.Access access) {
-                        throw new PythonThreadKillException();
-                    }
-                });
-                thread.interrupt();
+                killSystemThread(thread);
                 try {
                     thread.join(tries - i);
                 } catch (InterruptedException e) {
@@ -2343,6 +2336,16 @@ public Thread createSystemThread(PythonSystemThreadTask task) {
         return thread;
     }
 
+    public void killSystemThread(Thread thread) {
+        env.submitThreadLocal(new Thread[]{thread}, new ThreadLocalAction(true, false) {
+            @Override
+            protected void perform(ThreadLocalAction.Access access) {
+                throw new PythonThreadKillException();
+            }
+        });
+        thread.interrupt();
+    }
+
     public void initializeMainModule(TruffleString path) {
         if (path != null) {
             mainModule.setAttribute(T___FILE__, path);

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

@@ -42,7 +42,6 @@
 
 import com.oracle.truffle.api.TruffleSafepoint.Interruptible;
 import com.oracle.truffle.api.nodes.Node;
-import org.graalvm.polyglot.PolyglotException;
 
 /**
  * This exception kills a Python thread.
@@ -93,18 +92,4 @@ public PythonThreadKillException() {
     public final Throwable fillInStackTrace() {
         return this;
     }
-
-    /**
-     * Helper method for threads that do not run Python code and should gracefully terminate on
-     * catching {@link PythonThreadKillException} and some types of Polyglot exception thrown from
-     * Thread Location Actions run in Truffle Sefepoint.
-     */
-    public static boolean shouldKillThread(Throwable ex) {
-        if (ex instanceof PythonThreadKillException) {
-            return true;
-        } else if (ex instanceof PolyglotException pex) {
-            return pex.isCancelled() || pex.isInterrupted();
-        }
-        return false;
-    }
 }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/PythonSystemThreadTask.java

@@ -40,11 +40,11 @@
  */
 package com.oracle.graal.python.util;
 
+import java.util.logging.Level;
+
 import com.oracle.graal.python.runtime.exception.PythonThreadKillException;
 import com.oracle.truffle.api.TruffleLogger;
 
-import java.util.logging.Level;
-
 /**
  * Base class for runnables of "system" threads that handle some internal aspects of the Python
  * runtime. The thread should be created using
@@ -55,8 +55,9 @@
  * shutdown is through sending Java interrupt and submitting a Thread Local Action (TLA) that throws
  * {@link PythonThreadKillException}. The Java interrupt should interrupt any blocking operation,
  * which should then poll a safepoint. This wrapper takes care of handling
- * {@link PythonThreadKillException} gracefully as well as other exception types that Truffle itself
- * may submit through TLA to shut down the thread.
+ * {@link PythonThreadKillException} gracefully. Note that Truffle may also shut down this thread
+ * via TLA that throws its own internal cancellation exception, which is handled in Truffle internal
+ * code that wraps this Runnable.
  */
 public abstract class PythonSystemThreadTask implements Runnable {
     private final String name;
@@ -75,14 +76,15 @@ public String getName() {
     public final void run() {
         try {
             doRun();
+            logger.fine(() -> String.format("'%s' finished", name));
+        } catch (PythonThreadKillException ex) {
+            logger.fine(() -> String.format("%s killed with exception '%s'", name, PythonThreadKillException.class.getSimpleName()));
         } catch (Throwable ex) {
-            if (PythonThreadKillException.shouldKillThread(ex)) {
-                logger.fine(() -> String.format("%s killed with exception %s", name, ex));
-            } else {
-                logger.log(Level.WARNING, ex, () -> String.format("Unhandled exception %s in %s", ex.getClass().getSimpleName(), name));
-            }
+            // Note: Truffle wraps this Runnable in its own Runnable that handles Truffle's own
+            // internal cancellation exception that is submitted via TLA
+            logger.log(Level.FINE, String.format("Unhandled exception %s in thread '%s'", ex.getClass().getSimpleName(), name), ex);
+            throw ex;
         }
-        logger.fine(() -> String.format("%s finished", name));
     }
 
     protected abstract void doRun();