Re: [gui-dev] Startup deadlock problems

Jens-Uwe Mager <[email protected]> Fri, 23 Jul 2004 23:34:15 +0200
Newsgroups gmane.network.gnutella.limewire.gui.devel
Message-ID <[email protected]> 
On Fri, Jul 23, 2004 at 15:06 -0400, Sam Berlin wrote:

> Yeah, the startup is pretty fragile.  We'll gladly accept a patch that 
> puts the construction in the Swing thread & still manages to update the 
> splash screen. :)

OK, it does not need to be that an intrusive patch to get it working
properly. Sprinkleing a few SwingUtilities.invokeAndWait() into the
source does it. I did change the following three places that are prone
to the deadlock problem:

* SplashWindow: updating the status text modifies a realized window.
* StatusLine: same problem.
* GUIMediator.setAppVisible: making the main frame visible while other
  threads do updates to the GUI already (via SwingUtilities.invokeLater)
  produces a dead lock main thread against the Swing event dispatcher
  thread.

Especially the third one is the one that affected my additional main
tab, it implements a scrollable console window via BeanShell that may
already do prints to a console window. This is in particular visible if
logging via log4j is redirected to the console window.

The attached patch does indeed make the deadlock go away, but it appears
to have one side effect under Windows that I do not understand. The
code that does iconify LimeWire to the system tray does not
appear to work with my patch, it simply iconifies to the task bar. The
code appears to be highly magical, do you have any clues what might be
causing the breakage?

-- 
Jens-Uwe Mager	<pgp-mailto:F476EBC2>

_______________________________________________
gui-dev mailing list
[email protected]
http://www.limewire.org/mailman/listinfo/gui-dev
st.patch (text/plain, 3.9 KB) 
Index: com/limegroup/gnutella/gui/SplashWindow.java
===================================================================
RCS file: /cvs/gui/com/limegroup/gnutella/gui/SplashWindow.java,v
retrieving revision 1.26
diff -u -r1.26 SplashWindow.java
--- com/limegroup/gnutella/gui/SplashWindow.java	10 Jun 2004 19:11:11 -0000	1.26
+++ com/limegroup/gnutella/gui/SplashWindow.java	23 Jul 2004 21:08:50 -0000
@@ -1,6 +1,7 @@
 package com.limegroup.gnutella.gui;
 
 import com.limegroup.gnutella.settings.ThemeFileHandler;
+import com.limegroup.gnutella.ErrorService;
 
 import javax.swing.*;
 import java.awt.*;
@@ -59,8 +60,16 @@
      *
      * @param text the text to display
      */
-    static void setStatusText(String text) {
-        GLASS_PANE.setText(text);
+    static void setStatusText(final String text) {
+        try {
+        SwingUtilities.invokeAndWait(new Runnable() {
+            public void run() {
+                GLASS_PANE.setText(text);
+            }
+        });
+        } catch (Exception ex) {
+            ErrorService.error(ex);
+        }
     }
 }
 
Index: com/limegroup/gnutella/gui/StatusLine.java
===================================================================
RCS file: /cvs/gui/com/limegroup/gnutella/gui/StatusLine.java,v
retrieving revision 1.89
diff -u -r1.89 StatusLine.java
--- com/limegroup/gnutella/gui/StatusLine.java	21 May 2004 17:09:51 -0000	1.89
+++ com/limegroup/gnutella/gui/StatusLine.java	23 Jul 2004 21:08:51 -0000
@@ -4,6 +4,7 @@
 import java.awt.*;
 import javax.swing.*;
 import com.limegroup.gnutella.util.CommonUtils;
+import com.limegroup.gnutella.ErrorService;
 import com.limegroup.gnutella.gui.mp3.*;
 import com.limegroup.gnutella.settings.*;
 
@@ -160,10 +161,18 @@
      * Updates the status text.
      */
     void setStatusText(final String text) {
-        STATUS_LABEL.setText(text);
-        STATUS_LABEL.invalidate();
-        STATUS_LABEL.revalidate();
-        STATUS_LABEL.repaint();
+        try {
+            SwingUtilities.invokeAndWait(new Runnable() {
+            public void run() {
+                STATUS_LABEL.setText(text);
+                STATUS_LABEL.invalidate();
+                STATUS_LABEL.revalidate();
+                STATUS_LABEL.repaint();
+            }
+        });
+        } catch (Exception ex) {
+            ErrorService.error(ex);
+        }
     }
     
     /**
Index: com/limegroup/gnutella/gui/GUIMediator.java
===================================================================
RCS file: /cvs/gui/com/limegroup/gnutella/gui/GUIMediator.java,v
retrieving revision 1.136
diff -u -r1.136 GUIMediator.java
--- com/limegroup/gnutella/gui/GUIMediator.java	15 Jul 2004 15:47:50 -0000	1.136
+++ com/limegroup/gnutella/gui/GUIMediator.java	23 Jul 2004 21:08:53 -0000
@@ -2,6 +2,7 @@
 
 import com.limegroup.gnutella.RouterService;
 import com.limegroup.gnutella.bugs.BugManager;
+import com.limegroup.gnutella.ErrorService;
 import com.limegroup.gnutella.gui.library.LibraryMediator;
 import com.limegroup.gnutella.gui.notify.*;
 import com.limegroup.gnutella.gui.options.OptionsMediator;
@@ -311,9 +318,17 @@
 	 * @param visible specifies whether or not the application should be
 	 *                made visible or not
 	 */
-	public static final void setAppVisible(boolean visible) {
-		FRAME.setVisible(visible);
-		SearchMediator.requestSearchFocus();
+	public static final void setAppVisible(final boolean visible) {
+                try {
+                    SwingUtilities.invokeAndWait(new Runnable() {
+                        public void run() {
+                            FRAME.setVisible(visible);
+                            SearchMediator.requestSearchFocus();
+                        }
+                    });
+                } catch (Exception ex) {
+                    ErrorService.error(ex);
+                }
 		
 		// If the app has already been made visible, don't display extra
 		// dialogs.  We could display the pro dialog here, but it causes