Updated GTK threading model to *correctly* execute GTK methods on the

GTK thread. Solves problems (unsurprisingly) with race conditions
 against the native GTK thread.

src/dorkbox/systemTray/linux/GtkMenuEntry.java

@@ -79,13 +79,11 @@ class GtkMenuEntry implements MenuEntry, GCallback {
     int callback(final Pointer instance, final Pointer data) {
         final SystemTrayMenuAction cb = this.callback;
         if (cb != null) {
-            GtkTypeSystemTray.callbackExecutor.execute(new Runnable() {
+            try {
-                @Override
+                cb.onClick(parent, GtkMenuEntry.this);
-                public
+            } catch (Throwable throwable) {
-                void run() {
+                throwable.printStackTrace();
-                    cb.onClick(parent, GtkMenuEntry.this);
+            }
-                }
-            });
         }
 
         return Gtk.TRUE;

src/dorkbox/systemTray/linux/GtkSystemTray.java

@@ -21,6 +21,8 @@ import dorkbox.systemTray.linux.jna.Gobject;
 import dorkbox.systemTray.linux.jna.Gtk;
 import dorkbox.systemTray.linux.jna.GtkSupport;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
@@ -32,11 +34,8 @@ public
 class GtkSystemTray extends GtkTypeSystemTray {
     private Pointer trayIcon;
 
-    // have to make this a field, to prevent GC on this object
+    // have to save these in a field to prevent GC on the objects (since they go out-of-scope from java)
-    @SuppressWarnings("FieldCanBeLocal")
+    private final List<Object> gtkCallbacks = new ArrayList<Object>();
-    private Gobject.GEventCallback gtkCallback;
-    @SuppressWarnings({"FieldCanBeLocal", "unused"})
-    private NativeLong button_press_event;
 
     // This is required if we have JavaFX or SWT shutdown hooks (to prevent us from shutting down twice...)
     private AtomicBoolean shuttingDown = new AtomicBoolean();
@@ -53,12 +52,12 @@ class GtkSystemTray extends GtkTypeSystemTray {
             public
             void run() {
                 final Pointer trayIcon_ = gtk.gtk_status_icon_new();
-                gtk.gtk_status_icon_set_title(trayIcon_, "SystemTray@Dorkbox");
+                gtk.gtk_status_icon_set_title(trayIcon_, GnomeShellExtension.UID);
                 gtk.gtk_status_icon_set_name(trayIcon_, "SystemTray");
 
                 trayIcon = trayIcon_;
 
-                gtkCallback = new Gobject.GEventCallback() {
+                final Gobject.GEventCallback gtkCallback = new Gobject.GEventCallback() {
                     @Override
                     public
                     void callback(Pointer notUsed, final Gtk.GdkEventButton event) {
@@ -68,7 +67,11 @@ class GtkSystemTray extends GtkTypeSystemTray {
                         }
                     }
                 };
-                button_press_event = gobject.g_signal_connect_object(trayIcon, "button_press_event", gtkCallback, null, 0);
+                final NativeLong button_press_event = gobject.g_signal_connect_object(trayIcon, "button_press_event", gtkCallback, null, 0);
+
+                // have to do this to prevent GC on these objects
+                gtkCallbacks.add(gtkCallback);
+                gtkCallbacks.add(button_press_event);
             }
         });
     }
@@ -86,9 +89,9 @@ class GtkSystemTray extends GtkTypeSystemTray {
                     gtk.gtk_status_icon_set_visible(trayIcon, false);
                     gobject.g_object_unref(trayIcon);
 
-                    // GC it
+                    // mark for GC
                     trayIcon = null;
-                    gtkCallback = null;
+                    gtkCallbacks.clear();
                 }
             });
 

src/dorkbox/systemTray/linux/GtkTypeSystemTray.java

@@ -23,13 +23,9 @@ import dorkbox.systemTray.SystemTrayMenuAction;
 import dorkbox.systemTray.linux.jna.Gobject;
 import dorkbox.systemTray.linux.jna.Gtk;
 import dorkbox.systemTray.linux.jna.GtkSupport;
-import dorkbox.util.NamedThreadFactory;
 
 import java.io.InputStream;
 import java.net.URL;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Derived from
@@ -40,8 +36,6 @@ class GtkTypeSystemTray extends SystemTray {
     protected static final Gobject gobject = Gobject.INSTANCE;
     protected static final Gtk gtk = Gtk.INSTANCE;
 
-    final static ExecutorService callbackExecutor = Executors.newSingleThreadExecutor(new NamedThreadFactory("SysTrayExecutor", false));
-
     private volatile Pointer menu;
 
     private volatile Pointer connectionStatusItem;
@@ -62,16 +56,6 @@ class GtkTypeSystemTray extends SystemTray {
             void run() {
                 obliterateMenu();
 
-                boolean terminated = false;
-                try {
-                    terminated = callbackExecutor.awaitTermination(1, TimeUnit.SECONDS);
-                } catch (InterruptedException ignored) {
-                }
-
-                if (!terminated) {
-                    callbackExecutor.shutdownNow();
-                }
-
                 GtkSupport.shutdownGui();
             }
         });

src/dorkbox/systemTray/linux/jna/Gobject.java

@@ -15,9 +15,13 @@
  */
 package dorkbox.systemTray.linux.jna;
 
-import com.sun.jna.*;
+import com.sun.jna.Callback;
+import com.sun.jna.Library;
+import com.sun.jna.Native;
+import com.sun.jna.NativeLong;
+import com.sun.jna.Pointer;
+import com.sun.jna.Structure;
 import dorkbox.util.Keep;
-import dorkbox.systemTray.linux.jna.Gtk.GdkEventButton;
 
 import java.util.Arrays;
 import java.util.List;
@@ -126,6 +130,14 @@ interface Gobject extends Library {
     }
 
 
+    @Keep
+    interface FuncCallback extends Callback {
+        /**
+         * @return Gtk.FALSE if it will be automatically removed from the stack once it's handled
+         */
+        int callback(Pointer data);
+    }
+
     @Keep
     interface GCallback extends Callback {
         /**
@@ -137,7 +149,7 @@ interface Gobject extends Library {
 
     @Keep
     interface GEventCallback extends Callback {
-        void callback(Pointer instance, GdkEventButton event);
+        void callback(Pointer instance, Gtk.GdkEventButton event);
     }
 
 

src/dorkbox/systemTray/linux/jna/Gtk.java

@@ -66,8 +66,15 @@ interface Gtk extends Library {
      */
     void gtk_main();
 
+    /**
+     * using g_idle_add() instead would require thread protection in the callback
+     * @param callback
+     * @param data
+     * @return TRUE to run this callback again, FALSE to remove from the list of event sources (and not call it again)
+     */
+    int gdk_threads_add_idle (Gobject.FuncCallback callback, Pointer data);
 
-    /** sks for the current nesting level of the main loop. Useful to determine (at startup) if GTK is already runnign */
+    /** aks for the current nesting level of the main loop. Useful to determine (at startup) if GTK is already running */
     int gtk_main_level();
 
     /**

src/dorkbox/systemTray/linux/jna/GtkSupport.java

@@ -17,23 +17,31 @@ package dorkbox.systemTray.linux.jna;
 
 import com.sun.jna.Function;
 import com.sun.jna.Native;
+import com.sun.jna.Pointer;
 import dorkbox.systemTray.SystemTray;
 
-import java.util.concurrent.ArrayBlockingQueue;
+import java.util.LinkedList;
 import java.util.concurrent.CountDownLatch;
 
 public
 class GtkSupport {
+    // For funsies, SyncThing did a LOT of work on compatibility (unfortunate for us) in python.
+    // https://github.com/syncthing/syncthing-gtk/blob/b7a3bc00e3bb6d62365ae62b5395370f3dcc7f55/syncthing_gtk/statusicon.py
+
     private static volatile boolean started = false;
-    private static final ArrayBlockingQueue<Runnable> dispatchEvents = new ArrayBlockingQueue<Runnable>(256);
+
-    private static volatile Thread gtkDispatchThread;
+    // have to save these in a field to prevent GC on the objects (since they go out-of-scope from java)
+    private static final LinkedList<Object> gtkCallbacks = new LinkedList<Object>();
 
     /**
      * must call get() before accessing this! Only "Gtk" interface should access this!
      */
     static volatile Function gtk_status_icon_position_menu = null;
+
     public static volatile boolean isGtk2 = false;
+
     private static volatile boolean alreadyRunningGTK = false;
+    private static Thread gtkUpdateThread = null;
 
     /**
      * Helper for GTK, because we could have v3 or v2.
@@ -130,7 +138,7 @@ class GtkSupport {
         } catch (Throwable ignored) {
         }
 
-        throw new RuntimeException("We apologize for this, but we are unable to determine the GTK library is in use, if " +
+        throw new RuntimeException("We apologize for this, but we are unable to determine the GTK library is in use, " +
                                    "or even if it is in use... Please create an issue for this and include your OS type and configuration.");
     }
 
@@ -140,43 +148,14 @@ class GtkSupport {
         if (!started) {
             started = true;
 
-            // GTK specifies that we ONLY run from a single thread. This guarantees that.
-            gtkDispatchThread = new Thread() {
-                @Override
-                public
-                void run() {
-                    final Gtk gtk = Gtk.INSTANCE;
-                    while (started) {
-                        try {
-                            final Runnable take = dispatchEvents.take();
-
-                            gtk.gdk_threads_enter();
-
-                            try {
-                                take.run();
-                            } catch (Throwable e) {
-                                e.printStackTrace();
-                            }
-
-                            gtk.gdk_threads_leave();
-
-                        } catch (InterruptedException e) {
-                            e.printStackTrace();
-                        }
-                    }
-                }
-            };
-            gtkDispatchThread.setName("GTK Event Loop");
-            gtkDispatchThread.start();
-
-
             // startup the GTK GUI event loop. There can be multiple/nested loops.
 
             // If JavaFX/SWT is used, this is UNNECESSARY
             if (!alreadyRunningGTK) {
                 // only necessary if we are the only GTK instance running...
                 final CountDownLatch blockUntilStarted = new CountDownLatch(1);
-                Thread gtkUpdateThread = new Thread() {
+
+                gtkUpdateThread = new Thread() {
                     @Override
                     public
                     void run() {
@@ -191,6 +170,7 @@ class GtkSupport {
                         if (!SystemTray.COMPATIBILITY_MODE) {
                             gtk.gtk_init_check(0, null);
                         }
+
                         // notify our main thread to continue
                         blockUntilStarted.countDown();
 
@@ -202,7 +182,7 @@ class GtkSupport {
                         gtk.gdk_threads_leave();
                     }
                 };
-                gtkUpdateThread.setName("GTK Event Loop (Native)");
+                gtkUpdateThread.setName("GTK Native Event Loop");
                 gtkUpdateThread.start();
 
                 try {
@@ -216,40 +196,41 @@ class GtkSupport {
     }
 
     /**
-     * Best practices for GTK, is to call EVERYTHING for it on a SINGLE THREAD. This accomplishes that.
+     * Best practices for GTK, is to call EVERYTHING for it on the GTK THREAD. This accomplishes that.
      */
     public static
-    void dispatch(Runnable runnable) {
+    void dispatch(final Runnable runnable) {
-        try {
+        if (gtkUpdateThread == Thread.currentThread()) {
-            dispatchEvents.put(runnable);
+            // if we are ALREADY inside the native event
-        } catch (InterruptedException e) {
+            runnable.run();
-            e.printStackTrace();
+        } else {
+            final Gobject.FuncCallback callback = new Gobject.FuncCallback() {
+                @Override
+                public
+                int callback(final Pointer data) {
+                    synchronized (gtkCallbacks) {
+                        gtkCallbacks.removeFirst(); // now that we've 'handled' it, we can remove it from our callback list
+                    }
+                    runnable.run();
+
+                    return Gtk.FALSE; // don't want to call this again
+                }
+            };
+
+            synchronized (gtkCallbacks) {
+                gtkCallbacks.offer(callback); // prevent GC from collecting this object before it can be called
+            }
+            Gtk.INSTANCE.gdk_threads_add_idle(callback, null);
         }
     }
 
     public static
     void shutdownGui() {
-        // If JavaFX/SWT is used, this is UNNECESSARY (an will break SWT/JavaFX shutdown)
+        // If JavaFX/SWT is used, this is UNNECESSARY (and will break SWT/JavaFX shutdown)
         if (!(alreadyRunningGTK || SystemTray.COMPATIBILITY_MODE)) {
             Gtk.INSTANCE.gtk_main_quit();
         }
 
         started = false;
-
-        // put it in a NEW dispatch event (so that we cleanup AFTER this one is finished)
-        dispatch(new Runnable() {
-            @Override
-            public
-            void run() {
-                new Thread(new Runnable() {
-                    @Override
-                    public
-                    void run() {
-                        // this should happen in a new thread
-                        gtkDispatchThread.interrupt();
-                    }
-                }).run();
-            }
-        });
     }
 }