[m-rev.] diff: Fix a deadlock in the Java worker thread implementation

Paul Bone paul at bone.id.au
Tue Apr 12 15:08:56 AEST 2016


Fix a deadlock in the Java worker thread implementation

I noticed this bug when accidentally throwing a Mercury exception containing
some malformed Mercury data (a Java null pointer).  The Java worker thread
tried to report the exception, by calling back into Mercury but this
encountered a null pointer exception itself due to the bad data and failed
to de-register itself from the thread pool.  This caused the thread pool to
mistakingly think that a thread was still alive and it failed to terminate
the program.

The solution is tu ensure that the pool knows the thread has shutdown if any
exception occurs.

This bug isn't likely to occur in practice, throwing exceptions whose
message is null is unusual.

java/runtime/MercuryWorkerThread.java:
    As above.
---
 java/runtime/MercuryWorkerThread.java | 76 ++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/java/runtime/MercuryWorkerThread.java b/java/runtime/MercuryWorkerThread.java
index 994b30d..b5815c3 100644
--- a/java/runtime/MercuryWorkerThread.java
+++ b/java/runtime/MercuryWorkerThread.java
@@ -35,45 +35,47 @@ public class MercuryWorkerThread extends MercuryThread
     {
         Task task;
 
-        do {
-            task = null;
-            try {
-                if (status != ThreadStatus.IDLE) {
-                    setStatus(ThreadStatus.IDLE);
-                }
-                task = pool.workerGetTask();
-            }
-            catch (InterruptedException e) {
-                /*
-                ** A worker thread has no semantics for this, so we continue
-                ** looping.
-                */
-                continue;
-            }
-            if (task != null) {
+        try {
+            do {
+                task = null;
                 try {
-                    setStatus(ThreadStatus.WORKING);
-                    task.run();
-                    pool.taskDone(task);
-                } catch (jmercury.runtime.Exception e) {
-                    // The task threw a Mercury exception.
-                    pool.taskFailed(task, e);
-                    JavaInternal.reportUncaughtException(e);
-                    // Make the thread exit after throwing an exception.
-                    break;
-                } catch (Throwable e) {
-                    // Some other error occured. bail out.
-                    System.err.println("Uncaught exception: " + e.toString());
-                    System.err.println(e.getMessage());
-                    e.printStackTrace();
-                    System.exit(1);
-                } finally {
-                    setStatus(ThreadStatus.OTHER);
+                    if (status != ThreadStatus.IDLE) {
+                        setStatus(ThreadStatus.IDLE);
+                    }
+                    task = pool.workerGetTask();
                 }
-            }
-        } while (task != null);
-
-        pool.threadShutdown(this, status);
+                catch (InterruptedException e) {
+                    /*
+                    ** A worker thread has no semantics for this, so we continue
+                    ** looping.
+                    */
+                    continue;
+                }
+                if (task != null) {
+                    try {
+                        setStatus(ThreadStatus.WORKING);
+                        task.run();
+                        pool.taskDone(task);
+                    } catch (jmercury.runtime.Exception e) {
+                        // The task threw a Mercury exception.
+                        pool.taskFailed(task, e);
+                        JavaInternal.reportUncaughtException(e);
+                        // Make the thread exit after throwing an exception.
+                        break;
+                    } catch (Throwable e) {
+                        // Some other error occured. bail out.
+                        System.err.println("Uncaught exception: " + e.toString());
+                        System.err.println(e.getMessage());
+                        e.printStackTrace();
+                        System.exit(1);
+                    } finally {
+                        setStatus(ThreadStatus.OTHER);
+                    }
+                }
+            } while (task != null);
+        } finally {
+            pool.threadShutdown(this, status);
+        }
     }
 
     protected void setStatus(ThreadStatus new_status) {
-- 
2.8.0.rc3



More information about the reviews mailing list