Mercurial > hg > release > icedtea-web-1.0
changeset 18:d4a914a000e3
Stabilize plugin initialization
- Fixed frame pop-up issue when tab is closed early
- Fixed 100% CPU load when too many applets load in parallel
- Fixed message queue processing to prioritize destroy first
| author | Deepak Bhole <dbhole@redhat.com> |
|---|---|
| date | Tue, 26 Oct 2010 17:49:57 -0700 |
| parents | dfd749c077c3 |
| children | 98887d964b0e |
| files | ChangeLog plugin/icedteanp/java/sun/applet/PluginAppletViewer.java plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java |
| diffstat | 3 files changed, 263 insertions(+), 97 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog Tue Oct 26 12:01:22 2010 -0400 +++ b/ChangeLog Tue Oct 26 17:49:57 2010 -0700 @@ -1,3 +1,28 @@ +2010-10-26 Deepak Bhole <dbhole@redhat.com> + + * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java: + Replace all status.put calls with calls to updateStatus(). + (createPanel): Create a frame with a 0 handle. Use the new + waitForAppletInit function to wait until applet is ready. + (reFrame): Re-order code so that the panel is never parentless. + (handleMessage): Re-wrote message processing to handle destroy calls + correctly, checking for them more often to prevent a frame from popping up + if the tab/page is closed before loading finishes. Decode special + characters in the message. + (updateStatus): New function. Updates the status for the given instance if + applicable. + (destroyApplet): New function. Destroys a given applet and frees related + resources. + (waitForAppletInit): New function. Blocks until applet is initialized. + (parse): Remove part that decoded the params. Decoding is now done earlier + in handleMessage(). + * plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java: + (getPriorityStrIfPriority): Mark destroy messages as priority. + (bringPriorityMessagesToFront): Scans the queue for priority messages and + brings them to the front. + (run): If the queue is not empty and there are no workers left, run + bringPriorityMessagesToFront() and retry. + 2010-10-26 Andrew Su <asu@redhat.com> * Makefile.am: Split rm -rf into rm -f and rmdir for launcher
--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Tue Oct 26 12:01:22 2010 -0400 +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Tue Oct 26 17:49:57 2010 -0700 @@ -136,12 +136,10 @@ } } }); - + // create the frame. + PluginAppletViewer.reFrame(null, identifier, System.out, 0, panel); - // create the frame. - PluginAppletViewer.reFrame(null, identifier, System.out, handle, panel); - panel.init(); // Start the applet @@ -179,15 +177,7 @@ // Wait for the panel to initialize // (happens in a separate thread) - while (panel.getApplet() == null && - ((NetxPanel) panel).isAlive()) { - try { - Thread.sleep(50); - PluginDebug.debug("Waiting for applet to initialize..."); - } catch (InterruptedException ie) { - // just wait - } - } + PluginAppletViewer.waitForAppletInit((NetxPanel) panel); a = panel.getApplet(); @@ -317,7 +307,17 @@ */ private static String defaultSaveFile = "Applet.ser"; - private static enum PAV_INIT_STATUS {PRE_INIT, IN_INIT, INIT_COMPLETE, INACTIVE}; + + /** + * Enumerates the current status of an applet + * + * PRE_INIT -> Parsing and initialization phase + * INIT_COMPLETE -> Initialization complete, reframe pending + * REFRAME_COMPLETE -> Reframe complete, applet is initialized and usable by the user + * INACTIVE -> Browser has directed that the applet be destroyed (this state is non-overridable except by DESTROYED) + * DESTROYED -> Applet has been destroyed + */ + private static enum PAV_INIT_STATUS {PRE_INIT, INIT_COMPLETE, REFRAME_COMPLETE, INACTIVE, DESTROYED}; /** * The panel in which the applet is being displayed. @@ -350,7 +350,6 @@ private static HashMap<Integer, PAV_INIT_STATUS> status = new HashMap<Integer,PAV_INIT_STATUS>(); - private long handle = 0; private WindowListener windowEventListener = null; @@ -380,16 +379,20 @@ return; PluginAppletViewer newFrame = new PluginAppletViewer(handle, identifier, statusMsgStream, panel); - + if (oldFrame != null) { applets.remove(oldFrame.identifier); oldFrame.removeWindowListener(oldFrame.windowEventListener); panel.removeAppletListener(oldFrame.appletEventListener); + + // Add first, remove later + newFrame.add("Center", panel); oldFrame.remove(panel); oldFrame.dispose(); + } else { + newFrame.add("Center", panel); } - newFrame.add("Center", panel); newFrame.pack(); newFrame.appletEventListener = new AppletEventListener(newFrame, newFrame); @@ -397,11 +400,6 @@ applets.put(identifier, newFrame); - // dispose oldframe if necessary - if (oldFrame != null) { - oldFrame.dispose(); - } - PluginDebug.debug(panel + " reframed"); } @@ -449,7 +447,7 @@ this.frame = frame; this.appletViewer = appletViewer; } - + public void appletStateChanged(AppletEvent evt) { AppletPanel src = (AppletPanel)evt.getSource(); @@ -483,9 +481,9 @@ AppletPanel.changeFrameAppContext(frame, SunToolkit.targetToAppContext(a)); else AppletPanel.changeFrameAppContext(frame, AppContext.getAppContext()); - - status.put(appletViewer.identifier, PAV_INIT_STATUS.INIT_COMPLETE); - + + updateStatus(appletViewer.identifier, PAV_INIT_STATUS.INIT_COMPLETE); + break; } } @@ -510,7 +508,13 @@ try { if (message.startsWith("handle")) { - + + // If there is a key for this status, it means it + // was either initialized before, or destroy has been + // processed. Stop moving further. + if (updateStatus(identifier, PAV_INIT_STATUS.PRE_INIT) != null) + return; + // Extract the information from the message String[] msgParts = new String[4]; for (int i=0; i < 3; i++) { @@ -519,32 +523,38 @@ msgParts[i] = message.substring(spaceLocation + 1, nextSpaceLocation); message = message.substring(nextSpaceLocation + 1); } - + long handle = Long.parseLong(msgParts[0]); String width = msgParts[1]; String height = msgParts[2]; - + int spaceLocation = message.indexOf(' ', "tag".length()+1); String documentBase = UrlUtil.decode(message.substring("tag".length() + 1, spaceLocation)); - String tag = message.substring(spaceLocation+1); + String tag = message.substring(spaceLocation+1); + + // Decode the tag + tag = tag.replace(">", ">"); + tag = tag.replace("<", "<"); + tag = tag.replace("&", "&"); + tag = tag.replace(" ", "\n"); + tag = tag.replace(" ", "\r"); + tag = tag.replace(""", "\""); PluginDebug.debug ("Handle = " + handle + "\n" + "Width = " + width + "\n" + "Height = " + height + "\n" + "DocumentBase = " + documentBase + "\n" + "Tag = " + tag); - - status.put(identifier, PAV_INIT_STATUS.PRE_INIT); + PluginAppletViewer.parse (identifier, handle, width, height, new StringReader(tag), new URL(documentBase)); - int maxWait = APPLET_TIMEOUT; // wait for applet to fully load int wait = 0; - while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE) && + while ( !applets.containsKey(identifier) && // Map is populated only by reFrame (wait < maxWait)) { try { @@ -555,9 +565,44 @@ } } - if (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE)) + // If wait exceeded maxWait, we timed out. Throw an exception + if (wait >= maxWait) throw new Exception("Applet initialization timeout"); + PluginAppletViewer oldFrame = applets.get(identifier); + + // We should not try to destroy an applet during + // initialization. It may cause an inconsistent state, + // which would bad if it's a trusted applet that + // read/writes to files + waitForAppletInit((NetxPanel) applets.get(identifier).panel); + + // Should we proceed with reframing? + if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) { + destroyApplet(identifier); + return; + } + + // Proceed with re-framing + reFrame(oldFrame, identifier, System.out, handle, oldFrame.panel); + + // There is a slight chance that destroy can happen + // between the above and below line + if (updateStatus(identifier, PAV_INIT_STATUS.REFRAME_COMPLETE).equals(PAV_INIT_STATUS.INACTIVE)) { + destroyApplet(identifier); + } + + } else if (message.startsWith("destroy")) { + + // Set it inactive, and try to do cleanup is applicable + PAV_INIT_STATUS previousStatus = updateStatus(identifier, PAV_INIT_STATUS.INACTIVE); + PluginDebug.debug("Destroy status set for " + identifier); + + if (previousStatus != null && + previousStatus.equals(PAV_INIT_STATUS.REFRAME_COMPLETE)) { + destroyApplet(identifier); + } + } else { PluginDebug.debug ("Handling message: " + message + " instance " + identifier + " " + Thread.currentThread()); @@ -568,7 +613,7 @@ status.get(identifier).equals(PAV_INIT_STATUS.PRE_INIT) ) ); - + // don't bother processing further for inactive applets if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) return; @@ -580,31 +625,131 @@ e.printStackTrace(); // If an exception happened during pre-init, we need to update status - if (status.get(identifier).equals(PAV_INIT_STATUS.PRE_INIT)) - status.put(identifier, PAV_INIT_STATUS.INACTIVE); + updateStatus(identifier, PAV_INIT_STATUS.INACTIVE); throw new RuntimeException("Failed to handle message: " + message + " for instance " + identifier, e); } } - + + /** + * Sets the status unless an overriding status is set (e.g. if + * status is DESTROYED, it may not be overridden). + * + * @param identifier The identifier for which the status is to be set + * @param status The status to switch to + * @return The previous status + */ + private static synchronized PAV_INIT_STATUS updateStatus(int identifier, PAV_INIT_STATUS newStatus) { + + PAV_INIT_STATUS prev = status.get(identifier); + + // If the status is set + if (status.containsKey(identifier)) { + + // Nothing may override destroyed status + if (status.get(identifier).equals(PAV_INIT_STATUS.DESTROYED)) { + return prev; + } + + // If status is inactive, only DESTROYED may override it + if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) { + if (!newStatus.equals(PAV_INIT_STATUS.DESTROYED)) { + return prev; + } + } + } + + // Else set to given status + status.put(identifier, newStatus); + + return prev; + } + + /** + * Destroys the given applet instance. + * + * This function may be called multiple times without problems. + * It does a synchronized check on the status and will only + * attempt to destroy the applet if not previously destroyed. + * + * @param identifier The instance which is to be destroyed + */ + + private static synchronized void destroyApplet(int identifier) { + + PluginDebug.debug("DestroyApplet called for " + identifier); + + PAV_INIT_STATUS prev = updateStatus(identifier, PAV_INIT_STATUS.DESTROYED); + + // If already destroyed, return + if (prev.equals(PAV_INIT_STATUS.DESTROYED)) { + PluginDebug.debug(identifier + " already destroyed. Returning."); + return; + } + + // If already disposed, return + if (applets.get(identifier).panel.applet == null) { + // Try to still dispose the panel itself -- no harm done with double dispose + applets.get(identifier).dispose(); + + PluginDebug.debug(identifier + " inactive. Returning."); + return; + } + + PluginDebug.debug("Attempting to destroy " + identifier); + + final int fIdentifier = identifier; + SwingUtilities.invokeLater(new Runnable() { + public void run() { + applets.get(fIdentifier).appletClose(); + } + }); + + PluginDebug.debug(identifier + " destroyed"); + } + + /** + * Function to block until applet initialization is complete + * + * @param identifier The instance to wait for + */ + public static void waitForAppletInit(NetxPanel panel) { + + 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..."); + + Thread.sleep(waitTime += 50); + } catch (InterruptedException ie) { + // just wait + } + } + + PluginDebug.debug("Applet panel " + panel + " initialized"); + } + public void handleMessage(int reference, String message) { if (message.startsWith("width")) { // Wait for panel to come alive int maxWait = APPLET_TIMEOUT; // wait for panel to come alive - int wait = 0; + 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 - } - } - + try { + Thread.sleep(50); + wait += 50; + } catch (InterruptedException ie) { + // just wait + } + } // 0 => width, 1=> width_value, 2 => height, 3=> height_value String[] dimMsg = message.split(" "); @@ -636,7 +781,7 @@ panel.setSize(width, height); panel.validate(); - + panel.applet.resize(width, height); panel.applet.validate(); } @@ -649,9 +794,6 @@ e.printStackTrace(); } - } else if (message.startsWith("destroy")) { - dispose(); - status.put(identifier, PAV_INIT_STATUS.INACTIVE); } else if (message.startsWith("GetJavaObject")) { // FIXME: how do we determine what security context this @@ -672,15 +814,7 @@ // Wait for the panel to initialize // (happens in a separate thread) - while (panel.getApplet() == null && - ((NetxPanel) panel).isAlive()) { - try { - Thread.sleep(50); - PluginDebug.debug("Waiting for applet to initialize..."); - } catch (InterruptedException ie) { - // just wait - } - } + waitForAppletInit((NetxPanel) panel); PluginDebug.debug(panel + " -- " + panel.getApplet() + " -- " + ((NetxPanel) panel).isAlive()); @@ -1548,10 +1682,11 @@ if (countApplets() == 0) { appletSystemExit(); } + + updateStatus(identifier, PAV_INIT_STATUS.DESTROYED); } }).start(); - status.put(identifier, PAV_INIT_STATUS.INACTIVE); } /** @@ -1677,18 +1812,6 @@ val = buf.toString(); } - att = att.replace(">", ">"); - att = att.replace("<", "<"); - att = att.replace("&", "&"); - att = att.replace(" ", "\n"); - att = att.replace(" ", "\r"); - - val = val.replace(">", ">"); - val = val.replace("<", "<"); - val = val.replace("&", "&"); - val = val.replace(" ", "\n"); - val = val.replace(" ", "\r"); - PluginDebug.debug("PUT " + att + " = '" + val + "'"); atts.put(att.toLowerCase(java.util.Locale.ENGLISH), val); @@ -1708,7 +1831,6 @@ // private static final == inline private static final boolean isInt(Object o) { boolean isInt = false; - try { Integer.parseInt((String) o); isInt = true; @@ -1763,7 +1885,6 @@ } catch (IOException ioe) { return ioe; } - return null; } }; @@ -1785,6 +1906,7 @@ boolean isObjectTag = false; boolean isEmbedTag = false; boolean objectTagAlreadyParsed = false; + // The current character // FIXME: This is an evil hack to force pass-by-reference.. the // parsing code needs to be rewritten from scratch to prevent such @@ -1879,19 +2001,6 @@ if (val == null) { statusMsgStream.println(requiresNameWarning); } else if (atts != null) { - att = att.replace(">", ">"); - att = att.replace("<", "<"); - att = att.replace("&", "&"); - att = att.replace(" ", "\n"); - att = att.replace(" ", "\r"); - att = att.replace(""", "\""); - - val = val.replace(">", ">"); - val = val.replace("<", "<"); - val = val.replace("&", "&"); - val = val.replace(" ", "\n"); - val = val.replace(" ", "\r"); - val = val.replace(""", "\""); PluginDebug.debug("PUT " + att + " = " + val); atts.put(att.toLowerCase(), val); } else { @@ -1920,8 +2029,8 @@ if (atts.get("width") == null || !isInt(atts.get("width"))) { atts.put("width", width); - } - + } + if (atts.get("height") == null || !isInt(atts.get("height"))) { atts.put("height", height); } @@ -1974,7 +2083,7 @@ if (atts.get("width") == null || !isInt(atts.get("width"))) { atts.put("width", width); } - + if (atts.get("height") == null || !isInt(atts.get("height"))) { atts.put("height", height); } @@ -2023,10 +2132,11 @@ if (atts.get("width") == null || !isInt(atts.get("width"))) { atts.put("width", width); } - + if (atts.get("height") == null || !isInt(atts.get("height"))) { atts.put("height", height); } + } } }
--- a/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java Tue Oct 26 12:01:22 2010 -0400 +++ b/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java Tue Oct 26 17:49:57 2010 -0700 @@ -126,6 +126,10 @@ } private String getPriorityStrIfPriority(String message) { + + // Destroy messages are permanently high priority + if (message.endsWith("destroy")) + return "destroy"; synchronized (priorityWaitQueue) { Iterator<String> it = priorityWaitQueue.iterator(); @@ -150,7 +154,6 @@ } } - public void notifyWorkerIsFree(PluginMessageHandlerWorker worker) { synchronized (initWorkers) { Iterator<Integer> i = initWorkers.keySet().iterator(); @@ -176,6 +179,32 @@ } protected class ConsumerThread extends Thread { + + /** + * Scans the readQueue for priority messages and brings them to the front + */ + private void bringPriorityMessagesToFront() { + synchronized (readQueue) { + + // iterate through the list + for (int i=0; i < readQueue.size(); i++) { + + // remove element at i to inspect it + String message = readQueue.remove(i); + + // if element at i is a priority msg, bring it forward + if (getPriorityStrIfPriority(message) != null) { + readQueue.addFirst(message); + } else { // else keep it where it was + readQueue.add(i, message); + } + + // by the end the queue size is the same, so the + // position indicator (i) is still valid + } + } + } + public void run() { while (true) { @@ -190,7 +219,6 @@ String[] msgParts = message.split(" "); - String priorityStr = getPriorityStrIfPriority(message); boolean isPriorityResponse = (priorityStr != null); @@ -199,9 +227,12 @@ if (worker == null) { synchronized(readQueue) { - readQueue.addLast(message); + readQueue.addFirst(message); } + // re-scan to see if any priority message came in + bringPriorityMessagesToFront(); + continue; // re-loop to try next msg }