Don't interrupt message handling threads

3 files changed, 55 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index b2b4dd9..5852ee4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -13,6 +13,24 @@
 	* tests/cpp-unit-tests/IcedTeaParsePropertiesTest.cc: tests for library methods
 	* tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc: added tests for new methods
 	
+2013-03-28  Adam Domurad  <[email protected]>
+
+	Don't interrupt worker/consumer threads (can prevent shutdown code from
+	executing); instead use Object wait/notify methods.
+	* plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
+	(notifyHasWork): Replacement for thread interruption
+	(waitForWork): Replacement for thread sleeping
+	(run): Use waitForWork instead of Thread.sleep
+	(notifyWorkerIsFree): Removed -- misleading method.
+	* netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
+	(message): Make volatile, as it should have always been.
+	(notifyHasWork): Replacement for thread interruption
+	(waitForWork): Replacement for thread sleeping
+	(run): Use waitForWork instead of Thread.sleep
+	(getPermissions): avoid potential NPE if code source location is
+	missing
+	(free): Remove reference to notifyWorkerIsFree. 
+
 2013-03-26  Adam Domurad  <[email protected]>
 
 	Integration of unsigned applet confirmation dialogue.
diff --git a/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java b/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
index ec0a6e7..a5db7be 100644
--- a/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
+++ b/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
@@ -116,17 +116,13 @@ class PluginMessageConsumer {
         return null;
     }
 
-    public void notifyWorkerIsFree(PluginMessageHandlerWorker worker) {
-        consumerThread.interrupt();
-    }
-
     public void queue(String message) {
         synchronized (readQueue) {
             readQueue.addLast(message);
         }
 
         // Wake that lazy consumer thread
-        consumerThread.interrupt();
+        consumerThread.notifyHasWork();
     }
 
     protected class ConsumerThread extends Thread {
@@ -135,6 +131,22 @@ class PluginMessageConsumer {
             super("PluginMessageConsumer.ConsumerThread");
         }
 
+        // Notify that either work is ready to do, or a worker is available
+        public synchronized void notifyHasWork() {
+            notifyAll();
+        }
+
+        // Wait a bit until either work is ready to do, or a worker is available
+        public synchronized void waitForWork() {
+            try {
+                // Do not wait indefinitely to avoid the potential of deadlock
+                wait(1000);
+            } catch (InterruptedException e) {
+                // Should not typically occur
+                e.printStackTrace();
+            }
+        }
+
         /**
          * Scans the readQueue for priority messages and brings them to the front
          */
@@ -194,13 +206,10 @@ class PluginMessageConsumer {
                     }
 
                     worker.setmessage(message);
-                    worker.interrupt();
+                    worker.notifyHasWork();
 
                 } else {
-                    try {
-                        Thread.sleep(1000);
-                    } catch (InterruptedException ie) {
-                    }
+                    waitForWork();
                 }
             }
         }
diff --git a/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java b/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java
index 31e9e27..6115c53 100644
--- a/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java
+++ b/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java
@@ -42,10 +42,24 @@ class PluginMessageHandlerWorker extends Thread {
     private boolean free = true;
     private final boolean isPriorityWorker;
     private final int id;
-    private String message;
+    private volatile String message;
     private PluginStreamHandler streamHandler;
     private PluginMessageConsumer consumer;
 
+    public synchronized void notifyHasWork() {
+        notifyAll();
+    }
+
+    public synchronized void waitForWork() {
+        try {
+            // Do not wait indefinitely to avoid the potential of deadlock
+            wait(1000);
+        } catch (InterruptedException e) {
+            // Should not typically occur
+            e.printStackTrace();
+        }
+    }
+
     public PluginMessageHandlerWorker(
                 PluginMessageConsumer consumer,
                 PluginStreamHandler streamHandler, int id,
@@ -93,16 +107,10 @@ class PluginMessageHandlerWorker extends Thread {
                 free();
 
             } else {
+                waitForWork();
 
-                // Sleep when there is nothing to do
-                try {
-                    Thread.sleep(Integer.MAX_VALUE);
-                    PluginDebug.debug("Consumer thread ", id, " sleeping...");
-                } catch (InterruptedException ie) {
-                    PluginDebug.debug("Consumer thread ", id, " woken...");
-                    // nothing.. someone woke us up, see if there 
-                    // is work to do
-                }
+                // Someone woke us up, see if there is work to do
+                PluginDebug.debug("Consumer thread ", id, " woken...");
             }
         }
     }
@@ -120,9 +128,6 @@ class PluginMessageHandlerWorker extends Thread {
     public void free() {
         synchronized (this) {
             this.free = true;
-
-            // Signal the consumer that we are done in case it was waiting
-            consumer.notifyWorkerIsFree(this);
         }
     }