[icedtea-web] RFC: Switch to explicit locks and condition queues in PAV
Dr Andrew John Hughes
ahughes at redhat.com
Thu Apr 14 15:42:53 PDT 2011
This is the change I mentioned in the previous discussion. It replaces
the use of CHM monitors with explicit locks and condition queues. It also
simplifies (I hope) the panel lock case by using an explicit lock and
condition queue rather than a mix of the panel object and sleep.
I'm still a bit dubious about this class as a whole and this change
needs heavy testing before inclusion. Can someone try this out? I have
no idea how this stuff actually is run and tested sufficiently.
Long term this class needs a ground-up redesign. We're patching holes
here when this needs to go back to the drawing board IMHO. At present,
I have no overview of what this class is doing and it's far too big.
[Apologies in advance for a few whitespace issues in the patch. Clearly
someone has previously regressed on our identation cleanup and emacs
is auto-cleaning it up. I can easily drop these from the committed
version using -w if required].
ChangeLog:
2010-04-14 Andrew John Hughes <ahughes at 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.
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
-------------- next part --------------
diff -r b4db997469a2 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Thu Apr 14 21:41:32 2011 +0100
+++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Thu Apr 14 22:53:10 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();
@@ -263,12 +250,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;
@@ -276,6 +268,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;
@@ -309,11 +304,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");
}
@@ -364,9 +358,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: {
@@ -460,12 +454,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)
@@ -552,11 +551,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;
}
@@ -610,7 +609,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.
@@ -620,14 +619,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");
}
@@ -637,12 +640,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(" ");
@@ -691,21 +699,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
@@ -1370,7 +1375,7 @@
/**
* Decodes the string (converts html escapes into proper characters)
- *
+ *
* @param toDecode The string to decode
* @return The decoded string
*/
@@ -1385,7 +1390,7 @@
return toDecode;
}
-
+
/**
* System parameters.
*/
@@ -1571,7 +1576,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();
@@ -2060,34 +2065,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;
}
More information about the distro-pkg-dev
mailing list