changeset 215:c083edb1ce29

Switch to explicit locks and condition queues in PluginAppletViewer 2010-04-14 Andrew John Hughes <ahughes@redhat.com> * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java, (PluginAppletPanelFactory.createPanel(PluginStreamHandler, int,long,int,int,URL,Hashtable)): Remove duplication of wait for panel.isAlive(). (PluginAppletViewer.panelLock): New lock used to track panel creation. (PluginAppletViewer.panelLive): Condition queue for panel creation. (PluginAppletViewer.appletsLock): New lock used to track additions to the applets map. (PluginAppletViewer.appletAdded): Condition queue for applet addition. (PluginAppletViewer.statusLock): New lock for status changes. (PluginAppletViewer.initComplete): Condition queue for initialisation completion. (PluginAppletViewer.framePanel(int,long,NetxPanel)): Replace synchronized block with use of appletsLock and notification on appletAdded condition queue. (AppletEventListener.appletStateChanged(AppletEvent)): Signal the panelLive condition queue that the panel is live. (PluginAppletViewer.handleMessage(int,int,String)): Wait on appletAdded condition queue for applet to be added to the applets map. (PluginAppletViewer.updateStatus(Int,PAV_INIT_STATUS)): Signal when a status change occurs using the initComplete condition queue. (PluginAppletViewer.waitForAppletInit(NetxPanel)): Wait on the panelLive condition queue until the panel is created. (PluginAppletViewer.handleMessage(int,String)): Wait on the initComplete condition queue until initialisation is complete. Wait on the panelLive signal until panel is created. (waitTillTimeout(ReentrantLock,Condition,long)): Convert to use ReentrantLock and Condition. Add assertion to check the lock is held. Avoid conversion between milliseconds and nanoseconds.
author Andrew John Hughes <ahughes@redhat.com>
date Mon, 18 Apr 2011 18:44:40 +0100
parents bdb72bab9e9c
children 37f99d9fbdeb
files ChangeLog plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
diffstat 2 files changed, 107 insertions(+), 68 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Mon Apr 18 11:37:52 2011 -0400
+++ b/ChangeLog	Mon Apr 18 18:44:40 2011 +0100
@@ -1,3 +1,36 @@
+2010-04-14  Andrew John Hughes  <ahughes@redhat.com>
+
+	* plugin/icedteanp/java/sun/applet/PluginAppletViewer.java,
+	(PluginAppletPanelFactory.createPanel(PluginStreamHandler,
+	int,long,int,int,URL,Hashtable)): Remove duplication of wait
+	for panel.isAlive().
+	(PluginAppletViewer.panelLock): New lock used to track panel
+	creation.
+	(PluginAppletViewer.panelLive): Condition queue for panel creation.
+	(PluginAppletViewer.appletsLock): New lock used to track additions
+	to the applets map.
+	(PluginAppletViewer.appletAdded): Condition queue for applet addition.
+	(PluginAppletViewer.statusLock): New lock for status changes.
+	(PluginAppletViewer.initComplete): Condition queue for initialisation
+	completion.
+	(PluginAppletViewer.framePanel(int,long,NetxPanel)):
+	Replace synchronized block with use of appletsLock and notification
+	on appletAdded condition queue.
+	(AppletEventListener.appletStateChanged(AppletEvent)): Signal the
+	panelLive condition queue that the panel is live.
+	(PluginAppletViewer.handleMessage(int,int,String)): Wait on appletAdded
+	condition queue for applet to be added to the applets map.
+	(PluginAppletViewer.updateStatus(Int,PAV_INIT_STATUS)): Signal when a
+	status change occurs using the initComplete condition queue.
+	(PluginAppletViewer.waitForAppletInit(NetxPanel)): Wait on the panelLive
+	condition queue until the panel is created.
+	(PluginAppletViewer.handleMessage(int,String)): Wait on the initComplete
+	condition queue until initialisation is complete.  Wait on the panelLive
+	signal until panel is created.
+	(waitTillTimeout(ReentrantLock,Condition,long)): Convert to use
+	ReentrantLock and Condition.  Add assertion to check the lock is held.
+	Avoid conversion between milliseconds and nanoseconds.
+
 2011-04-18  Deepak Bhole <dbhole@redhat.com>
 
 	* plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Mon Apr 18 11:37:52 2011 -0400
+++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java	Mon Apr 18 18:44:40 2011 +0100
@@ -97,8 +97,13 @@
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Vector;
+
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.TimeUnit;
+
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
 
 import javax.swing.SwingUtilities;
 
@@ -139,25 +144,7 @@
         // Start the applet
         initEventQueue(panel);
 
-        // Wait for panel to come alive
-        // Wait implemented the long way so that
-        // PluginAppletViewer.waitTillTimeout() needn't be exposed.
-        long maxTimeToSleep = PluginAppletViewer.APPLET_TIMEOUT/1000000; // ns -> ms
-
-        synchronized(panel) {
-            while (!panel.isAlive() && maxTimeToSleep > 0) {
-                long sleepStart = System.nanoTime();
-
-                try {
-                    panel.wait(maxTimeToSleep);
-                } catch (InterruptedException e) {} // Just loop back
-
-                maxTimeToSleep -= (System.nanoTime() - sleepStart)/1000000; // ns -> ms
-            }
-        }
-
         // Wait for the panel to initialize
-        // (happens in a separate thread)
         PluginAppletViewer.waitForAppletInit(panel);
 
         Applet a = panel.getApplet();
@@ -287,12 +274,17 @@
      * The panel in which the applet is being displayed.
      */
     private NetxPanel panel;
-
+    static final ReentrantLock panelLock = new ReentrantLock();
+    // CONDITION PREDICATE: panel.isAlive()
+    static final Condition panelLive = panelLock.newCondition();
     private int identifier;
 
     // Instance identifier -> PluginAppletViewer object.
     private static ConcurrentMap<Integer, PluginAppletViewer> applets =
             new ConcurrentHashMap<Integer, PluginAppletViewer>();
+    private static final ReentrantLock appletsLock = new ReentrantLock();
+    // CONDITION PREDICATE: !applets.containsKey(identifier)
+    private static final Condition appletAdded = appletsLock.newCondition();
 
     private static PluginStreamHandler streamhandler;
 
@@ -300,6 +292,9 @@
 
     private static ConcurrentMap<Integer, PAV_INIT_STATUS> status =
             new ConcurrentHashMap<Integer, PAV_INIT_STATUS>();
+    private static final ReentrantLock statusLock = new ReentrantLock();
+    // CONDITION PREDICATE: !status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE)
+    private static final Condition initComplete = statusLock.newCondition();
 
     private WindowListener windowEventListener = null;
     private AppletEventListener appletEventListener = null;
@@ -333,11 +328,10 @@
         appletFrame.appletEventListener = new AppletEventListener(appletFrame, appletFrame);
         panel.addAppletListener(appletFrame.appletEventListener);
 
-        
-        synchronized(applets) {
-            applets.put(identifier, appletFrame);
-            applets.notifyAll();
-        }
+        appletsLock.lock();
+        applets.put(identifier, appletFrame);
+        appletAdded.signalAll();
+        appletsLock.unlock();
 
         PluginDebug.debug(panel, " framed");
     }
@@ -388,9 +382,9 @@
         public void appletStateChanged(AppletEvent evt) {
             AppletPanel src = (AppletPanel) evt.getSource();
 
-            synchronized (src) {
-                src.notifyAll();
-            }
+            panelLock.lock();
+            panelLive.signalAll();
+            panelLock.unlock();
 
             switch (evt.getID()) {
                 case AppletPanel.APPLET_RESIZE: {
@@ -484,12 +478,17 @@
                                                 new URL(documentBase));
 
                 long maxTimeToSleep = APPLET_TIMEOUT;
-                synchronized(applets) {
+                appletsLock.lock();
+                try {
                     while (!applets.containsKey(identifier) &&
                             maxTimeToSleep > 0) { // Map is populated only by reFrame
-                        maxTimeToSleep -= waitTillTimeout(applets, maxTimeToSleep);
+                        maxTimeToSleep -= waitTillTimeout(appletsLock, appletAdded,
+                                                          maxTimeToSleep);
                     }
                 }
+                finally {
+                    appletsLock.unlock();
+                }
 
                 // If wait exceeded maxWait, we timed out. Throw an exception
                 if (maxTimeToSleep <= 0)
@@ -576,11 +575,11 @@
         }
 
         // Else set to given status
-        
-        synchronized(status) {
-            status.put(identifier, newStatus);
-            status.notifyAll();
-        }
+
+        statusLock.lock();
+        status.put(identifier, newStatus);
+        initComplete.signalAll();
+        statusLock.unlock();
 
         return prev;
     }
@@ -634,7 +633,7 @@
 
     /**
      * Function to block until applet initialization is complete.
-     * 
+     *
      * This function will return if the wait is longer than {@link #APPLET_TIMEOUT}
      *
      * @param panel the instance to wait for.
@@ -644,14 +643,18 @@
         // Wait till initialization finishes
         long maxTimeToSleep = APPLET_TIMEOUT;
 
-        synchronized(panel) {
+        panelLock.lock();
+        try {
             while (panel.getApplet() == null &&
                     panel.isAlive() &&
                     maxTimeToSleep > 0) {
                 PluginDebug.debug("Waiting for applet panel ", panel, " to initialize...");
-                maxTimeToSleep -= waitTillTimeout(panel, maxTimeToSleep);
+                maxTimeToSleep -= waitTillTimeout(panelLock, panelLive, maxTimeToSleep);
             }
         }
+        finally {
+            panelLock.unlock();
+        }
 
         PluginDebug.debug("Applet panel ", panel, " initialized");
     }
@@ -661,12 +664,17 @@
 
             // Wait for panel to come alive
             long maxTimeToSleep = APPLET_TIMEOUT;
-            synchronized(status) {
+            statusLock.lock();
+            try {
                 while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE) &&
                         maxTimeToSleep > 0) {
-                    maxTimeToSleep -= waitTillTimeout(status, maxTimeToSleep);
+                    maxTimeToSleep -= waitTillTimeout(statusLock, initComplete,
+                                                      maxTimeToSleep);
                 }
             }
+            finally {
+                statusLock.unlock();
+            }
 
             // 0 => width, 1=> width_value, 2 => height, 3=> height_value
             String[] dimMsg = message.split(" ");
@@ -715,21 +723,18 @@
             // FIXME: how do we determine what security context this
             // object should belong to?
             Object o;
-            
+
             // First, wait for panel to instantiate
-            int waited = 0;
-            while (panel == null && waited < APPLET_TIMEOUT) {
-                try {
-                    Thread.sleep(50);
-                    waited += 50;
-                } catch (InterruptedException ie) {} // discard, loop back
-            }
-
             // Next, wait for panel to come alive
             long maxTimeToSleep = APPLET_TIMEOUT;
-            synchronized(panel) {
-                while (!panel.isAlive())
-                    maxTimeToSleep -= waitTillTimeout(panel, maxTimeToSleep);
+            panelLock.lock();
+            try {
+                while (panel == null || !panel.isAlive())
+                    maxTimeToSleep -= waitTillTimeout(panelLock, panelLive,
+                                                      maxTimeToSleep);
+            }
+            finally {
+                panelLock.unlock();
             }
 
             // Wait for the panel to initialize
@@ -1394,7 +1399,7 @@
 
     /**
      * Decodes the string (converts html escapes into proper characters)
-     * 
+     *
      * @param toDecode The string to decode
      * @return The decoded string
      */
@@ -1409,7 +1414,7 @@
 
         return toDecode;
     }
-    
+
     /**
      * System parameters.
      */
@@ -1595,7 +1600,7 @@
             public void run() {
                 ClassLoader cl = p.applet.getClass().getClassLoader();
 
-                // Since we want to deal with JNLPClassLoader, extract it if this 
+                // Since we want to deal with JNLPClassLoader, extract it if this
                 // is a codebase loader
                 if (cl instanceof JNLPClassLoader.CodeBaseClassLoader)
                     cl = ((JNLPClassLoader.CodeBaseClassLoader) cl).getParentJNLPClassLoader();
@@ -2084,34 +2089,35 @@
     }
 
     /**
-     * Waits on a given object until timeout. 
-     * 
+     * Waits on a given condition queue until timeout.
+     *
      * <b>This function assumes that the monitor lock has already been
      * acquired by the caller.</b>
-     * 
-     * If the given object is null, this function returns immediately.
-     * 
-     * @param obj The object to wait on
+     *
+     * If the given lock is null, this function returns immediately.
+     *
+     * @param lock the lock that must be held when this method is called.
+     * @param cond the condition queue on which to wait for notifications.
      * @param timeout The maximum time to wait (nanoseconds)
      * @return Approximate time spent sleeping (not guaranteed to be perfect)
      */
-    public static long waitTillTimeout(Object obj, long timeout) {
+    public static long waitTillTimeout(ReentrantLock lock, Condition cond,
+                                       long timeout) {
 
         // Can't wait on null. Return 0 indicating no wait happened.
-        if (obj == null)
+        if (lock == null)
             return 0;
-        
+
+        assert lock.isHeldByCurrentThread();
+
         // Record when we started sleeping
         long sleepStart = 0L;
 
-        // Convert timeout to ms since wait() takes ms
-        timeout = timeout >= 1000000 ? timeout/1000000 : 1; 
-
         try {
             sleepStart = System.nanoTime();
-            obj.wait(timeout);
+            cond.await(timeout, TimeUnit.NANOSECONDS);
         } catch (InterruptedException ie) {} // Discarded, time to return
-        
+
         // Return the difference
         return System.nanoTime() - sleepStart;
     }