[m-rev.] diff: [java] Fix non-termination when main/2 throws an exception

[Paul Bone]

Branches: master (the bug only exists on master).

---
[java] Fix non-termination when main/2 throws an exception

When main/2 throws an exception we did not properly shutdown the thread pool
and therefore the JVM would not shut down.  Simply calling shutdown() in a
finally block is insufficient because then the primordial thread may finish
before the worker thread is able to report the exception thrown by main/2.
This doesn't seem right because the JVM is supposed to wait for all the
non-daemon threads to finish before it exits.  I suspect that the primordial
thread is closing stdout and stderr as it exits and therefore the exception
is never seen, but I don't know.

This change fixes the issue by ensuring that shutdown() is always called (in
a finally block) and that the main thread waits for the thread pool to
shutdown before it exits.

java/runtime/MercuryThreadPool.java:
    runMain() will not exit until the worker threads have exited.

    Create a new method waitForShutdown() that will wait for the thread pool
    to shutdown.

    Signal the main thread when a worker thread exits.

java/runtime/MercuryWorkerThread.java:
    Worker threads now exit if their task raises an unhanded exception.

java/runtime/MercuryRuntime.java:
    Allow standalone programs to have the same behavour as programs whose
    entrypoint is written in Mercury.
---
 java/runtime/MercuryRuntime.java      |  7 ++-
 java/runtime/MercuryThreadPool.java   | 87 +++++++++++++++++++++++++++++------
 java/runtime/MercuryWorkerThread.java |  2 +
 3 files changed, 82 insertions(+), 14 deletions(-)

diff --git a/java/runtime/MercuryRuntime.java b/java/runtime/MercuryRuntime.java
index a50b9f6..b3b8391 100644
--- a/java/runtime/MercuryRuntime.java
+++ b/java/runtime/MercuryRuntime.java
@@ -50,10 +50,15 @@ public class MercuryRuntime
      * Finalise the runtime system.
      * This _must_ be called at the normal end of any program.  It runs
      * finalisers and stops the thread pool.
+     * This will wait for the thread pool to shutdown.
      */
     public static void finalise() {
+        MercuryThreadPool pool;
+
+        pool = getThreadPool();
         JavaInternal.run_finalisers();
-        getThreadPool().shutdown();
+        pool.shutdown();
+        pool.waitForShutdown();
     }
 }
 
diff --git a/java/runtime/MercuryThreadPool.java b/java/runtime/MercuryThreadPool.java
index 4133907..676f2ef 100644
--- a/java/runtime/MercuryThreadPool.java
+++ b/java/runtime/MercuryThreadPool.java
@@ -311,6 +311,8 @@ public class MercuryThreadPool
         } finally {
             threads_lock.unlock();
         }
+
+        signalMainLoop();
     }
 
     public void taskDone(Task task)
@@ -346,7 +348,12 @@ public class MercuryThreadPool
     {
         main_loop_lock.lock();
         try {
-            main_loop_condition.signal();
+            /*
+             * There may be more than one thread waiting on this condition
+             * such as when more than one thread calls waitForShutdown().
+             * I can't imagine this happening, bit it is allowed.
+             */
+            main_loop_condition.signalAll();
         } finally {
             main_loop_lock.unlock();
         }
@@ -407,8 +414,7 @@ public class MercuryThreadPool
              */
             threads_lock.lock();
             try {
-                int num_threads = num_threads_working + num_threads_waiting +
-                    num_threads_blocked + num_threads_other;
+                int num_threads = numThreads();
                 int num_threads_limit = thread_pool_size +
                     num_threads_blocked;
                 // Determine the number of new threads that we want.
@@ -452,6 +458,15 @@ public class MercuryThreadPool
     }
 
     /**
+     * Get the total number of threads.
+     * Caller must hold threads lock.
+     */
+    private int numThreads() {
+        return num_threads_working + num_threads_waiting +
+            num_threads_blocked + num_threads_other;
+    }
+
+    /**
      * Run the thread pool.
      * The calling thread is used to "run" the thread pool.  Its main job
      * is to keep the correct number of worker threads alive.  It does not
@@ -542,20 +557,56 @@ public class MercuryThreadPool
             }
         }
 
-        /*
-         * Shutdown
-         */
+        doShutdown();
+    }
+
+    /**
+     * Perform the shutdown.
+     */
+    private void doShutdown()
+    {
         tasks_lock.lock();
         try {
             shutdown_now = true;
-            thread_wait_for_task_condition.signalAll();
             running = false;
+            thread_wait_for_task_condition.signalAll();
         } finally {
             tasks_lock.unlock();
         }
     }
 
     /**
+     * Wait for all the worker threads to exit.
+     * Even though the JVM is not supposed to exit until all the non-daemon
+     * threads have exited the effects of some threads may get lost.  I
+     * suspect this may be because the main thread closes stdout and stderr.
+     * Waiting for the worker threads fixes the problem - pbone.
+     */
+    public boolean waitForShutdown()
+    {
+        boolean has_shutdown = false;
+
+        if (shutdown_request) {
+            do {
+                main_loop_lock.lock();
+                try {
+                    has_shutdown = numThreads() == 0;
+                    if (!has_shutdown) {
+                        main_loop_condition.await();
+                    }
+                } catch (InterruptedException e) {
+                    continue;
+                } finally {
+                    main_loop_lock.unlock();
+                }
+            } while (!has_shutdown);
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    /**
      * Start the thread pool in its own thread.
      * Normally the thread pool ie executed directly by the main thread.
      * However, when Mercury is used as a library by a native Java
@@ -589,12 +640,13 @@ public class MercuryThreadPool
 
     /**
      * Request that the thread pool shutdown.
-     * This method does not wait for the thread pool to shutdown, it's an
-     * asynchronous signal.  The thread pool will shutdown if: shutdown() has
+     * The thread pool will shutdown if: shutdown() has
      * been called (implicitly when main/2 is written in Mercury) and there
      * are no remaining tasks either queued or running (spawn_native tasks
-     * are not included).  The requirement that the process does not exit
-     * until all tasks have finish is maintained by the JVM.
+     * are not included).
+     *
+     * This method is asynchronous, it will not wait for the thread pool to
+     * shutdown.
      */
     public boolean shutdown()
     {
@@ -624,8 +676,11 @@ public class MercuryThreadPool
 
         run_main_and_shutdown = new Runnable() {
             public void run() {
-                run_main.run();
-                shutdown();
+                try {
+                    run_main.run();
+                } finally {
+                    shutdown();
+                }
             }
         };
         main_task = new Task(run_main_and_shutdown);
@@ -649,6 +704,12 @@ public class MercuryThreadPool
              */
             run();
             jmercury.runtime.JavaInternal.run_finalisers();
+            /*
+             * We always wait for the thread pool to shutdown as worker
+             * threads may either be completing work or reporting the reason
+             * why the runtime is aborting.
+             */
+            waitForShutdown();
         } catch (jmercury.runtime.Exception e) {
             JavaInternal.reportUncaughtException(e);
         }
diff --git a/java/runtime/MercuryWorkerThread.java b/java/runtime/MercuryWorkerThread.java
index a3d0d4f..994b30d 100644
--- a/java/runtime/MercuryWorkerThread.java
+++ b/java/runtime/MercuryWorkerThread.java
@@ -59,6 +59,8 @@ public class MercuryWorkerThread extends MercuryThread
                     // 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());
-- 
2.1.3