[icedtea-web] RFC: Patch to use wait() when waiting for conditions to become true
Jiri Vanek
jvanek at redhat.com
Thu Apr 14 03:25:27 PDT 2011
On 04/14/2011 07:56 AM, Deepak Bhole wrote:
> * Deepak Bhole<dbhole at redhat.com> [2011-04-13 20:48]:
>> > Hi,
>> >
>> > Andrew mentioned in one of the previous threads that the current plugin
>> > code waits in an inefficient manner. It wakes up periodically to
>> > check for conditions, and goes back to sleep, and keeps doing that
>> > cyclically.
>> >
>> > The attached patch uses a new function that uses object.wait for
>> > the object of interest.
>> >
> Please see attached new patch. After leaving the office I realized that
> there is a potential race condition in the old patch with locking the
> object inside the wait function. Patch updated to do the locking
> outside (and function comment updated to clearly state that).
>
> Cheers,
> Deepak
>
>
> wait-fixes.patch
>
>
> diff -r 2ef65ff10619 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Wed Apr 13 19:17:21 2011 -0400
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Thu Apr 14 01:54:04 2011 -0400
> @@ -91,12 +91,10 @@
> import java.security.PrivilegedAction;
> import java.security.PrivilegedActionException;
> import java.security.PrivilegedExceptionAction;
> -import java.util.ArrayList;
> import java.util.Enumeration;
> import java.util.HashMap;
> import java.util.Hashtable;
> import java.util.Iterator;
> -import java.util.List;
> import java.util.Map;
> import java.util.Vector;
> import java.util.concurrent.ConcurrentHashMap;
> @@ -142,14 +140,19 @@
> initEventQueue(panel);
>
> // Wait for panel to come alive
> - int maxWait = PluginAppletViewer.APPLET_TIMEOUT; // wait for panel to come alive
> - int wait = 0;
> - while (!panel.isAlive()&& wait< maxWait) {
> - try {
> - Thread.sleep(50);
> - wait += 50;
> - } catch (InterruptedException ie) {
> - // just wait
> + // Wait implemented the long way so that
> + // PluginAppletViewer.waitTillTimeout() needn't be exposed.
> + long maxTimeToSleep = PluginAppletViewer.APPLET_TIMEOUT;
> +
> + synchronized(panel) {
> + while (!panel.isAlive()&& maxTimeToSleep> 0) {
> + long sleepStart = System.currentTimeMillis();
> +
> + try {
> + panel.wait(maxTimeToSleep);
> + } catch (InterruptedException e) {} // Just loop back
> +
> + maxTimeToSleep -= (System.currentTimeMillis() - sleepStart);
> }
> }
>
> @@ -307,6 +310,10 @@
> panel.addAppletListener(appletFrame.appletEventListener);
>
> applets.put(identifier, appletFrame);
> +
> + synchronized(applets) {
> + applets.notifyAll();
> + }
>
> PluginDebug.debug(panel, " framed");
> }
> @@ -357,6 +364,10 @@
> public void appletStateChanged(AppletEvent evt) {
> AppletPanel src = (AppletPanel) evt.getSource();
>
> + synchronized (src) {
> + src.notifyAll();
> + }
> +
> switch (evt.getID()) {
> case AppletPanel.APPLET_RESIZE: {
> if (src != null) {
> @@ -448,21 +459,16 @@
> new StringReader(tag),
> new URL(documentBase));
>
> - int maxWait = APPLET_TIMEOUT; // wait for applet to fully load
> - int wait = 0;
> - while (!applets.containsKey(identifier)&& // Map is populated only by reFrame
> - (wait< maxWait)) {
> -
> - try {
> - Thread.sleep(50);
> - wait += 50;
> - } catch (InterruptedException ie) {
> - // just wait
> + long maxTimeToSleep = APPLET_TIMEOUT;
> + synchronized(applets) {
> + while (!applets.containsKey(identifier)&&
> + maxTimeToSleep> 0) { // Map is populated only by reFrame
> + maxTimeToSleep -= waitTillTimeout(applets, maxTimeToSleep);
> }
> }
>
> // If wait exceeded maxWait, we timed out. Throw an exception
> - if (wait>= maxWait)
> + if (maxTimeToSleep<= 0)
> throw new Exception("Applet initialization timeout");
>
> // We should not try to destroy an applet during
> @@ -547,6 +553,10 @@
>
> // Else set to given status
> status.put(identifier, newStatus);
> +
> + synchronized(status) {
> + status.notifyAll();
> + }
>
> return prev;
> }
> @@ -599,7 +609,9 @@
> }
>
> /**
> - * Function to block until applet initialization is complete
> + * 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.
> */
> @@ -608,16 +620,14 @@
> int waitTime = 0;
>
> // Wait till initialization finishes
> - while (panel.getApplet() == null&&
> - panel.isAlive()&&
> - waitTime< APPLET_TIMEOUT) {
> - try {
> - if (waitTime % 500 == 0)
> - PluginDebug.debug("Waiting for applet panel ", panel, " to initialize...");
> + long maxTimeToSleep = APPLET_TIMEOUT;
>
> - Thread.sleep(waitTime += 50);
> - } catch (InterruptedException ie) {
> - // just wait
> + synchronized(panel) {
> + while (panel.getApplet() == null&&
> + panel.isAlive()&&
> + maxTimeToSleep> 0) {
> + PluginDebug.debug("Waiting for applet panel ", panel, " to initialize...");
> + maxTimeToSleep -= waitTillTimeout(panel, maxTimeToSleep);
> }
> }
>
> @@ -628,14 +638,11 @@
> if (message.startsWith("width")) {
>
> // Wait for panel to come alive
> - int maxWait = APPLET_TIMEOUT; // wait for panel to come alive
> - int wait = 0;
> - while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE)&& wait< maxWait) {
> - try {
> - Thread.sleep(50);
> - wait += 50;
> - } catch (InterruptedException ie) {
> - // just wait
> + int maxTimeToSleep = APPLET_TIMEOUT;
> + synchronized(status) {
> + while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE)&&
> + maxTimeToSleep> 0) {
> + maxTimeToSleep -= waitTillTimeout(status, maxTimeToSleep);
> }
> }
>
> @@ -686,17 +693,21 @@
> // FIXME: how do we determine what security context this
> // object should belong to?
> Object o;
> -
> - // Wait for panel to come alive
> - int maxWait = APPLET_TIMEOUT; // wait for panel to come alive
> - int wait = 0;
> - while ((panel == null) || (!panel.isAlive()&& wait< maxWait)) {
> +
> + // First, wait for panel to instantiate
> + int waited = 0;
> + while (panel == null&& waited< APPLET_TIMEOUT) {
> try {
> Thread.sleep(50);
> - wait += 50;
> - } catch (InterruptedException ie) {
> - // just wait
> - }
> + 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);
> }
>
> // Wait for the panel to initialize
> @@ -2049,4 +2060,36 @@
> // Draw the painted image
> g.drawImage(bufFrameImg, 0, 0, this);
> }
> +
> + /**
> + * Waits on a given object 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
> + * @param timeout The maximum time to wait
> + * @return Approximate time spent sleeping (not guaranteed to be perfect)
> + */
> + public static long waitTillTimeout(Object obj, long timeout) {
> +
> + // Can't wait on null. Return 0 indicating no wait happened.
> + if (obj == null)
> + return 0;
> +
> + // Record when we started sleeping
> + long sleepStart = System.currentTimeMillis();
> +
> + try {
> + obj.wait(timeout);
> + } catch (InterruptedException ie) {} // Discarded, time to return
> +
> + // Record when we woke up
> + long sleepEnd = System.currentTimeMillis();
> +
> + // Return the difference
> + return sleepEnd - sleepStart;
> + }
> }
I'm expecting Andrew will say something about missing changelog(!) or
something like that O:) But I have found no problem here.
For me push OK.
regards J.
More information about the distro-pkg-dev
mailing list