[cp-patches] RFC: revert single thread GTK event queue design

[Thomas Fitzsimmons]

Hi,

This patch reverts EventQueue and the GTK peers from its current single-
threaded design to a simpler two-threaded design.  The GTK main loop
will now run in its own thread, separate from the AWT event dispatch
thread.  I'm reverting this for two reasons:

One, the contention between the AWT event queue lock and the GDK lock was 
impossible to resolve satisfactorily.  The single threaded design was only 
workable if we could release the GDK lock in GTK callbacks that posted to the 
event queue (thus acquiring the AWT event queue lock).  It turns out that it is 
unsafe to release the GDK lock in that situation.  Therefore our current 
callbacks are wrong, and the assumption that the single threaded design relied 
on is invalid.

Two, the single-threaded design introduced new methods to
ClasspathToolkit and calls to those methods in EventQueue.  With the
introduction of the Qt peers, there's a push to remove references to
ClasspathToolkit from within the java.awt namespace.

Comments?

Tom

2005-08-15  Thomas Fitzsimmons  <address@hidden>

        * gnu/java/awt/ClasspathToolkit.java (nativeQueueEmpty): Remove
        method.
        (wakeNativeQueue): Likewise.
        (iterateNativeQueue): Likewise.
        * gnu/java/awt/peer/gtk/GtkToolkit.java (static): Start GTK main
        thread.
        (nativeQueueEmpty): Remove method.
        (wakeNativeQueue): Likewise.
        (iterateNativeQueue): Likewise.
        (gtkMain): New method.
        * include/gnu_java_awt_peer_gtk_GtkToolkit.h: Regenerate.
        * java/awt/EventQueue.java: Remove references to ClasspathToolkit.
        * java/awt/Frame.java (fireDummyEvent): Remove method.
        Remove calls to fireDummyEvent.
        * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkGenericPeer.c
        (dispose): Don't wake up main thread.
        * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c: Remove
        single-thread code.

Index: gnu/java/awt/ClasspathToolkit.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/ClasspathToolkit.java,v
retrieving revision 1.13
diff -u -r1.13 ClasspathToolkit.java
--- gnu/java/awt/ClasspathToolkit.java  14 Aug 2005 20:36:58 -0000      1.13
+++ gnu/java/awt/ClasspathToolkit.java  15 Aug 2005 04:14:56 -0000
@@ -46,7 +46,6 @@
 import java.awt.AWTException;
 import java.awt.Dimension;
 import java.awt.DisplayMode;
-import java.awt.EventQueue;
 import java.awt.Font;
 import java.awt.FontMetrics;
 import java.awt.GraphicsDevice;
@@ -171,9 +170,4 @@
    * @param w The embedded window with which to associate a peer.
    */
   public abstract EmbeddedWindowPeer createEmbeddedWindow (EmbeddedWindow w);
-
-  public abstract boolean nativeQueueEmpty();
-  public abstract void wakeNativeQueue();  
-  public abstract void iterateNativeQueue(EventQueue locked, boolean block);
 }
-
Index: gnu/java/awt/peer/gtk/GtkToolkit.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/peer/gtk/GtkToolkit.java,v
retrieving revision 1.72
diff -u -r1.72 GtkToolkit.java
--- gnu/java/awt/peer/gtk/GtkToolkit.java       14 Aug 2005 20:36:57 -0000      
1.72
+++ gnu/java/awt/peer/gtk/GtkToolkit.java       15 Aug 2005 04:14:56 -0000
@@ -124,6 +124,14 @@
 
     // Register ImageIO SPIs
     GdkPixbufDecoder.registerSpis( IIORegistry.getDefaultInstance() );
+
+    new Thread ("GTK main thread")
+    {
+      public void run ()
+      {
+       gtkMain ();
+      }
+    }.start ();
   }
 
   public GtkToolkit ()
@@ -644,8 +652,5 @@
     return new GdkRobotPeer (screen);
   }
 
-  public native boolean nativeQueueEmpty();
-  public native void wakeNativeQueue();  
-  public native void iterateNativeQueue(EventQueue locked, boolean block);
-
+  public static native void gtkMain();
 } // class GtkToolkit
Index: include/gnu_java_awt_peer_gtk_GtkToolkit.h
===================================================================
RCS file: 
/cvsroot/classpath/classpath/include/gnu_java_awt_peer_gtk_GtkToolkit.h,v
retrieving revision 1.8
diff -u -r1.8 gnu_java_awt_peer_gtk_GtkToolkit.h
--- include/gnu_java_awt_peer_gtk_GtkToolkit.h  18 Jan 2005 09:43:45 -0000      
1.8
+++ include/gnu_java_awt_peer_gtk_GtkToolkit.h  15 Aug 2005 04:14:57 -0000
@@ -16,9 +16,7 @@
 JNIEXPORT jint JNICALL 
Java_gnu_java_awt_peer_gtk_GtkToolkit_getScreenResolution (JNIEnv *env, 
jobject);
 JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkToolkit_sync (JNIEnv 
*env, jobject);
 JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkToolkit_loadSystemColors 
(JNIEnv *env, jobject, jintArray);
-JNIEXPORT jboolean JNICALL 
Java_gnu_java_awt_peer_gtk_GtkToolkit_nativeQueueEmpty (JNIEnv *env, jobject);
-JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkToolkit_wakeNativeQueue 
(JNIEnv *env, jobject);
-JNIEXPORT void JNICALL 
Java_gnu_java_awt_peer_gtk_GtkToolkit_iterateNativeQueue (JNIEnv *env, jobject, 
jobject, jboolean);
+JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkToolkit_gtkMain (JNIEnv 
*env, jclass);
 
 #ifdef __cplusplus
 }
Index: java/awt/EventQueue.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/EventQueue.java,v
retrieving revision 1.23
diff -u -r1.23 EventQueue.java
--- java/awt/EventQueue.java    6 Jul 2005 08:15:17 -0000       1.23
+++ java/awt/EventQueue.java    15 Aug 2005 04:14:57 -0000
@@ -38,8 +38,6 @@
 
 package java.awt;
 
-import gnu.java.awt.ClasspathToolkit;
-
 import java.awt.event.ActionEvent;
 import java.awt.event.InputEvent;
 import java.awt.event.InputMethodEvent;
@@ -78,9 +76,6 @@
   private EventDispatchThread dispatchThread = new EventDispatchThread(this);
   private boolean shutdown = false;
 
-  private long lastNativeQueueAccess = 0;
-  private long humanLatencyThreshold = 100;
-
   synchronized void setShutdown (boolean b) 
   {
     shutdown = b;
@@ -94,8 +89,8 @@
     // This is the exact self-shutdown condition specified in J2SE:
     // 
http://java.sun.com/j2se/1.4.2/docs/api/java/awt/doc-files/AWTThreadIssues.html
     
-    if (peekEvent() == null
-        && ((ClasspathToolkit) Toolkit.getDefaultToolkit()).nativeQueueEmpty())
+    // FIXME: check somewhere that the native queue is empty
+    if (peekEvent() == null)
       {
         Frame[] frames = Frame.getFrames();
         for (int i = 0; i < frames.length; ++i)
@@ -127,23 +122,9 @@
   {
     if (next != null)
       return next.getNextEvent();
-    
-    ClasspathToolkit tk = ((ClasspathToolkit) Toolkit.getDefaultToolkit());
-    long curr = System.currentTimeMillis();
-
-    if (! tk.nativeQueueEmpty() &&
-        (curr - lastNativeQueueAccess > humanLatencyThreshold))
-      {
-        tk.iterateNativeQueue(this, false);
-        lastNativeQueueAccess = curr;
-      }
 
     while (next_in == next_out)
       {
-        // Only the EventDispatchThread associated with the top of the stack is
-        // allowed to get events from the native source; everyone else just
-        // waits on the head of the queue.
-
         if (isDispatchThread())
           {
             // We are not allowed to return null from this method, yet it
@@ -158,19 +139,8 @@
             if (isShutdown())
               throw new InterruptedException();
 
-            tk.iterateNativeQueue(this, true);
-            lastNativeQueueAccess = System.currentTimeMillis();
-          }
-        else
-          {
-            try
-              {
-                wait();
-              }
-            catch (InterruptedException ie)
-              {
-              }
-          }
+           wait();
+         }
       }
 
     AWTEvent res = queue[next_out];
@@ -298,15 +268,6 @@
         dispatchThread.start();
       }
 
-    // Window events might represent the closing of a window, which
-    // might cause the end of the dispatch thread's life, so we'll wake
-    // it up here to give it a chance to check for shutdown.
-
-    if (!isDispatchThread() 
-        || (evt.getID() == WindowEvent.WINDOW_CLOSED)
-        || (evt.getID() == WindowEvent.WINDOW_CLOSING))
-      ((ClasspathToolkit) Toolkit.getDefaultToolkit()).wakeNativeQueue();
-
     notify();
   }
 
@@ -478,7 +439,6 @@
            next_in = 0;
            next_out = 0;
 
-            ((ClasspathToolkit) Toolkit.getDefaultToolkit()).wakeNativeQueue();
             setShutdown(true);
            dispatchThread = null;
             this.notifyAll();
Index: java/awt/Frame.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Frame.java,v
retrieving revision 1.32
diff -u -r1.32 Frame.java
--- java/awt/Frame.java 2 Jul 2005 20:32:24 -0000       1.32
+++ java/awt/Frame.java 15 Aug 2005 04:14:57 -0000
@@ -401,20 +401,6 @@
   menuBar.remove(menu);
 }
 
-/**
-  * Notifies this frame that it should create its native peer.
-  */
-private static void fireDummyEvent()
-{
-  EventQueue.invokeLater(new Runnable()
-    {
-      public void run()
-      {
-       // Do nothing here.
-      }
-    });
-}
-
 public void
 addNotify()
 {
@@ -423,11 +409,6 @@
   if (peer == null)
     peer = getToolkit ().createFrame (this);
 
-  // We now know there's a Frame (us) with a live peer, so we can start the
-  // fundamental queue and dispatch thread, by inserting a dummy event.
-  if (parent != null && parent.isDisplayable())
-    fireDummyEvent();
-  
   super.addNotify();
 }
 
@@ -436,12 +417,6 @@
   if (menuBar != null)
     menuBar.removeNotify();
   super.removeNotify();
-
-  // By now we've been disconnected from the peer, and the peer set to
-  // null.  This is formally the same as saying "we just became
-  // un-displayable", so we wake up the event queue with a dummy event to
-  // see if it's time to shut down.
-  fireDummyEvent();
 }
 
   /**
Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkGenericPeer.c
===================================================================
RCS file: 
/cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkGenericPeer.c,v
retrieving revision 1.8
diff -u -r1.8 gnu_java_awt_peer_gtk_GtkGenericPeer.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkGenericPeer.c  14 Jul 2005 
22:07:02 -0000      1.8
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkGenericPeer.c  15 Aug 2005 
04:14:57 -0000
@@ -58,13 +58,6 @@
   NSA_DEL_PTR (env, obj);
 
   gdk_threads_leave ();
-
-  /* 
-   * Wake up the main thread, to make sure it re-checks the window
-   * destruction condition. 
-   */
-
-  g_main_context_wakeup (NULL);
 }
 
 JNIEXPORT void JNICALL
Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c
===================================================================
RCS file: 
/cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c,v
retrieving revision 1.19
diff -u -r1.19 gnu_java_awt_peer_gtk_GtkToolkit.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c      14 Jul 2005 
22:07:02 -0000      1.19
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c      15 Aug 2005 
04:14:57 -0000
@@ -41,8 +41,6 @@
 #include "gthread-jni.h"
 #include "jcl.h"
 
-#include <sys/time.h>
-
 #define RC_FILE ".classpath-gtkrc"
 
 /* From java.awt.SystemColor */
@@ -292,20 +290,6 @@
       PANGO_SCALE * 72.0 / (int_dpi / PANGO_SCALE);
 }
 
-static int
-within_human_latency_tolerance(struct timeval *init)
-{
-  struct timeval curr;
-  unsigned long milliseconds_elapsed;
-
-  gettimeofday(&curr, NULL);
-  
-  milliseconds_elapsed = (((curr.tv_sec * 1000) + (curr.tv_usec / 1000))
-                         - ((init->tv_sec * 1000) + (init->tv_usec / 1000)));
-  
-  return milliseconds_elapsed < 100;
-}
-
 #if GTK_MINOR_VERSION > 4
 static void
 glog_func (const gchar *log_domain,
@@ -332,54 +316,15 @@
 }
 #endif
 
-JNIEXPORT void JNICALL 
-Java_gnu_java_awt_peer_gtk_GtkToolkit_iterateNativeQueue
-(JNIEnv *env, 
- jobject self __attribute__((unused)),
- jobject lockedQueue,
- jboolean block)
+JNIEXPORT void JNICALL
+Java_gnu_java_awt_peer_gtk_GtkToolkit_gtkMain
+(JNIEnv *env __attribute__((unused)), jobject obj __attribute__((unused)))
 {
-  /* We're holding an EventQueue lock, and we're about to acquire the GDK
-   * lock before dropping the EventQueue lock. This can deadlock if someone
-   * holds the GDK lock and wants to acquire the EventQueue lock; however
-   * all callbacks from GTK happen with the GDK lock released, so this
-   * would only happen in an odd case such as some JNI helper code
-   * acquiring the GDK lock and calling back into
-   * EventQueue.getNextEvent().
-   */
-
-  struct timeval init;
-  gettimeofday(&init, NULL);
-
   gdk_threads_enter ();
-  (*env)->MonitorExit (env, lockedQueue);
 
-  if (block)
-    {
-      
-      /* If we're blocking-when-empty, we want a do .. while loop. */
-      do 
-       gtk_main_iteration ();
-      while (within_human_latency_tolerance (&init) 
-            && gtk_events_pending ());
-    }
-  else
-    {
-      /* If we're not blocking-when-empty, we want a while loop. */
-      while (within_human_latency_tolerance (&init) 
-            && gtk_events_pending ())
-       gtk_main_iteration ();      
-    }
-  
-  (*env)->MonitorEnter (env, lockedQueue);
-  gdk_threads_leave ();
-}
+  gtk_main ();
 
-JNIEXPORT void JNICALL 
-Java_gnu_java_awt_peer_gtk_GtkToolkit_wakeNativeQueue
-  (JNIEnv *env __attribute__((unused)), jobject obj __attribute__((unused)))
-{
-  g_main_context_wakeup (NULL);
+  gdk_threads_leave ();
 }
 
 JNIEXPORT jboolean JNICALL