[icedtea-web] RFC: Patch to fix PR778

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-10-20 14:24]:
> On 10/20/2011 01:50 PM, Deepak Bhole wrote:
> >Hi,
> >
> >This is a patch to fix PR778 -
> >"Jar download and server certificate verification deadlock":
> >http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=778
> >
> >ChangeLog:
> >2011-10-20  Deepak Bhole<dbhole at redhat.com>
> >
> >     PR778: Jar download and server certificate verification deadlock
> >     * netx/net/sourceforge/jnlp/GuiLaunchHandler.java (launchInitialized):
> >     Moved as much code as possible out of the invokeLater block.
> >
> >Unfortunately, the reproducer from the site has gone off-line and I was unable
> >to create one. I did however try this patch with the original reproducer and it
> >worked fine.
> >
> >Even without a reproducer, it makes sense from a logical standpoint to have as
> >little as possible in an invokeLater call.
> 
> Yes, it does make sense. I do have a concern though.
> JNLPSplashScreen extends JDialog, so I am not sure if doing the
> equivalent of "new JDialog()" on a non-EDT thread is safe. The docs
> aren't very clear on this, but here is a blog that discusses the
> issue:
> http://bitguru.wordpress.com/2007/03/21/will-the-real-swing-single-threading-rule-please-stand-up/
> 

Interesting. I wasn't aware that constructor calls needed to happen in
the EDT. Nice catch!

Modified patch attached.

Cheers,
Deepak
-------------- next part --------------
diff -r f1468696eda3 netx/net/sourceforge/jnlp/GuiLaunchHandler.java
--- a/netx/net/sourceforge/jnlp/GuiLaunchHandler.java	Mon Oct 17 18:41:42 2011 +0200
+++ b/netx/net/sourceforge/jnlp/GuiLaunchHandler.java	Thu Oct 20 16:42:18 2011 -0400
@@ -37,6 +37,7 @@
 
 package net.sourceforge.jnlp;
 
+import java.lang.reflect.InvocationTargetException;
 import java.net.URL;
 
 import javax.swing.SwingUtilities;
@@ -95,20 +96,40 @@
 
     @Override
     public void launchInitialized(final JNLPFile file) {
+        
+        int preferredWidth = 500;
+        int preferredHeight = 400;
+
+        final URL splashImageURL = file.getInformation().getIconLocation(
+                IconDesc.SPLASH, preferredWidth, preferredHeight);
+
+        if (splashImageURL != null) {
+            final ResourceTracker resourceTracker = new ResourceTracker(true);
+            resourceTracker.addResource(splashImageURL, file.getFileVersion(), null, policy);
+            synchronized(mutex) {
+                try {
+                    SwingUtilities.invokeAndWait(new Runnable() {
+                        @Override
+                        public void run() {
+                            splashScreen = new JNLPSplashScreen(resourceTracker, null, null);
+                        }
+                    });
+                } catch (InterruptedException ie) {
+                    // Wait till splash screen is created
+                    while (splashScreen == null);
+                } catch (InvocationTargetException ite) {
+                    // Do nothing
+                }
+
+                splashScreen.setSplashImageURL(splashImageURL);
+            }
+        }
+        
         SwingUtilities.invokeLater(new Runnable() {
             @Override
             public void run() {
-                final int preferredWidth = 500;
-                final int preferredHeight = 400;
-
-                URL splashImageURL = file.getInformation().getIconLocation(
-                        IconDesc.SPLASH, preferredWidth, preferredHeight);
                 if (splashImageURL != null) {
-                    ResourceTracker resourceTracker = new ResourceTracker(true);
-                    resourceTracker.addResource(splashImageURL, file.getFileVersion(), null, policy);
                     synchronized(mutex) {
-                        splashScreen = new JNLPSplashScreen(resourceTracker, null, null);
-                        splashScreen.setSplashImageURL(splashImageURL);
                         if (splashScreen.isSplashScreenValid()) {
                             splashScreen.setVisible(true);
                         }