From 6fdbe8ac8364a9457388b60434b626518f15f5ec Mon Sep 17 00:00:00 2001 From: nathan Date: Tue, 5 Apr 2016 14:07:41 +0200 Subject: [PATCH] Abstracted 'dispatch(Runnable)' so that all updates to the SystemTray occur on the dispatch thread. This will resolve any race condition issues when creating, then (before it's actually created) trying to modify a menu entry. --- src/dorkbox/systemTray/SystemTray.java | 355 ++++++++++++++---- .../systemTray/linux/AppIndicatorTray.java | 6 +- .../systemTray/linux/GtkSystemTray.java | 6 +- .../systemTray/linux/GtkTypeSystemTray.java | 7 +- .../systemTray/swing/SwingSystemTray.java | 13 +- 5 files changed, 301 insertions(+), 86 deletions(-) diff --git a/src/dorkbox/systemTray/SystemTray.java b/src/dorkbox/systemTray/SystemTray.java index 263f552..99ac5d3 100644 --- a/src/dorkbox/systemTray/SystemTray.java +++ b/src/dorkbox/systemTray/SystemTray.java @@ -41,6 +41,9 @@ import java.net.URL; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Iterator; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; /** @@ -430,6 +433,11 @@ class SystemTray { SystemTray() { } + /** + * Necessary to guarantee all updates occur on the dispatch thread + */ + protected abstract + void dispatch(Runnable runnable); /** * Must be wrapped in a synchronized block for object visibility @@ -593,16 +601,37 @@ class SystemTray { * @param newMenuText the new menu text (this will replace the original menu text) */ public final - void updateMenuEntry_Text(String origMenuText, String newMenuText) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(origMenuText); + void updateMenuEntry_Text(final String origMenuText, final String newMenuText) { + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry == null) { - throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); - } - else { - menuEntry.setText(newMenuText); + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(origMenuText); + + if (menuEntry == null) { + hasValue.set(false); + } + else { + menuEntry.setText(newMenuText); + } + } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); } } @@ -613,16 +642,37 @@ class SystemTray { * @param imagePath the new path for the image to use or null to delete the image */ public final - void updateMenuEntry_Image(String origMenuText, String imagePath) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(origMenuText); + void updateMenuEntry_Image(final String origMenuText, final String imagePath) { + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry == null) { - throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); - } - else { - menuEntry.setImage(imagePath); + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(origMenuText); + + if (menuEntry == null) { + hasValue.set(false); + } + else { + menuEntry.setImage(imagePath); + } + } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); } } @@ -633,16 +683,38 @@ class SystemTray { * @param imageUrl the new URL for the image to use or null to delete the image */ public final - void updateMenuEntry_Image(String origMenuText, URL imageUrl) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(origMenuText); + void updateMenuEntry_Image(final String origMenuText, final URL imageUrl) { + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry == null) { - throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); - } - else { - menuEntry.setImage(imageUrl); + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(origMenuText); + + if (menuEntry == null) { + hasValue.set(false); + + } + else { + menuEntry.setImage(imageUrl); + } + } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); } } @@ -653,16 +725,37 @@ class SystemTray { * @param imageStream the InputStream of the image to use or null to delete the image */ public final - void updateMenuEntry_Image(String origMenuText, String cacheName, InputStream imageStream) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(origMenuText); + void updateMenuEntry_Image(final String origMenuText, final String cacheName, final InputStream imageStream) { + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry == null) { - throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); - } - else { - menuEntry.setImage(cacheName, imageStream); + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(origMenuText); + + if (menuEntry == null) { + hasValue.set(false); + } + else { + menuEntry.setImage(cacheName, imageStream); + } + } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); } } @@ -675,18 +768,39 @@ class SystemTray { * @param origMenuText the original menu text * @param imageStream the new path for the image to use or null to delete the image */ - @Deprecated public final - void updateMenuEntry_Image(String origMenuText, InputStream imageStream) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(origMenuText); + void updateMenuEntry_Image(final String origMenuText, final InputStream imageStream) { + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry == null) { - throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); - } - else { - menuEntry.setImage(imageStream); + dispatch(new Runnable() { + @SuppressWarnings("deprecation") + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(origMenuText); + + if (menuEntry == null) { + hasValue.set(false); + } + else { + menuEntry.setImage(imageStream); + } + } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); } } @@ -697,16 +811,37 @@ class SystemTray { * @param newCallback the new callback (this will replace the original callback) */ public final - void updateMenuEntry_Callback(String origMenuText, SystemTrayMenuAction newCallback) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(origMenuText); + void updateMenuEntry_Callback(final String origMenuText, final SystemTrayMenuAction newCallback) { + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry != null) { - menuEntry.setCallback(newCallback); - } - else { - throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(origMenuText); + + if (menuEntry == null) { + hasValue.set(false); + } + else { + menuEntry.setCallback(newCallback); + } + } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); } } @@ -719,17 +854,38 @@ class SystemTray { * @param newCallback the new callback (this will replace the original callback) */ public final - void updateMenuEntry(String origMenuText, String newMenuText, SystemTrayMenuAction newCallback) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(origMenuText); + void updateMenuEntry(final String origMenuText, final String newMenuText, final SystemTrayMenuAction newCallback) { + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry == null) { - throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); - } - else { - menuEntry.setText(newMenuText); - menuEntry.setCallback(newCallback); + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(origMenuText); + + if (menuEntry == null) { + hasValue.set(false); + } + else { + menuEntry.setText(newMenuText); + menuEntry.setCallback(newCallback); + } + } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + origMenuText + "'"); } } @@ -747,20 +903,42 @@ class SystemTray { final String label = menuEntry.getText(); - synchronized (menuEntries) { - for (Iterator iterator = menuEntries.iterator(); iterator.hasNext(); ) { - final MenuEntry entry = iterator.next(); - if (entry.getText() - .equals(label)) { - iterator.remove(); + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(false); - // this will also reset the menu - menuEntry.remove(); - return; + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + for (Iterator iterator = menuEntries.iterator(); iterator.hasNext(); ) { + final MenuEntry entry = iterator.next(); + if (entry.getText() + .equals(label)) { + iterator.remove(); + + // this will also reset the menu + menuEntry.remove(); + hasValue.set(true); + countDownLatch.countDown(); + return; + } + } } + countDownLatch.countDown(); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("Menu entry '" + label + "'not found in list while trying to remove it."); } - throw new NullPointerException("Menu entry '" + label + "'not found in list while trying to remove it."); } @@ -771,15 +949,42 @@ class SystemTray { */ public final void removeMenuEntry(final String menuText) { - synchronized (menuEntries) { - MenuEntry menuEntry = getMenuEntry(menuText); + // have to wait for the value + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicBoolean hasValue = new AtomicBoolean(true); - if (menuEntry == null) { - throw new NullPointerException("No menu entry exists for string '" + menuText + "'"); - } - else { - removeMenuEntry(menuEntry); + dispatch(new Runnable() { + @Override + public + void run() { + dispatch(new Runnable() { + @Override + public + void run() { + synchronized (menuEntries) { + MenuEntry menuEntry = getMenuEntry(menuText); + + if (menuEntry == null) { + hasValue.set(false); + } + else { + removeMenuEntry(menuEntry); + } + } + countDownLatch.countDown(); + } + }); } + }); + + try { + final boolean await = countDownLatch.await(2, TimeUnit.SECONDS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + if (!hasValue.get()) { + throw new NullPointerException("No menu entry exists for string '" + menuText + "'"); } } } diff --git a/src/dorkbox/systemTray/linux/AppIndicatorTray.java b/src/dorkbox/systemTray/linux/AppIndicatorTray.java index ddb9b0b..e5bed2b 100644 --- a/src/dorkbox/systemTray/linux/AppIndicatorTray.java +++ b/src/dorkbox/systemTray/linux/AppIndicatorTray.java @@ -43,7 +43,7 @@ class AppIndicatorTray extends GtkTypeSystemTray { AppIndicatorTray() { GtkSupport.startGui(); - GtkSupport.dispatch(new Runnable() { + dispatch(new Runnable() { @Override public void run() { @@ -57,7 +57,7 @@ class AppIndicatorTray extends GtkTypeSystemTray { public void shutdown() { if (!shuttingDown.getAndSet(true)) { - GtkSupport.dispatch(new Runnable() { + dispatch(new Runnable() { @Override public void run() { @@ -77,7 +77,7 @@ class AppIndicatorTray extends GtkTypeSystemTray { @Override protected void setIcon_(final String iconPath) { - GtkSupport.dispatch(new Runnable() { + dispatch(new Runnable() { @Override public void run() { diff --git a/src/dorkbox/systemTray/linux/GtkSystemTray.java b/src/dorkbox/systemTray/linux/GtkSystemTray.java index 7b31d3c..1bb2cae 100644 --- a/src/dorkbox/systemTray/linux/GtkSystemTray.java +++ b/src/dorkbox/systemTray/linux/GtkSystemTray.java @@ -48,7 +48,7 @@ class GtkSystemTray extends GtkTypeSystemTray { super(); GtkSupport.startGui(); - GtkSupport.dispatch(new Runnable() { + dispatch(new Runnable() { @Override public void run() { @@ -78,7 +78,7 @@ class GtkSystemTray extends GtkTypeSystemTray { public void shutdown() { if (!shuttingDown.getAndSet(true)) { - GtkSupport.dispatch(new Runnable() { + dispatch(new Runnable() { @Override public void run() { @@ -99,7 +99,7 @@ class GtkSystemTray extends GtkTypeSystemTray { @Override protected void setIcon_(final String iconPath) { - GtkSupport.dispatch(new Runnable() { + dispatch(new Runnable() { @Override public void run() { diff --git a/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java b/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java index f10c41b..589c14b 100644 --- a/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java +++ b/src/dorkbox/systemTray/linux/GtkTypeSystemTray.java @@ -47,6 +47,11 @@ class GtkTypeSystemTray extends SystemTray { private volatile Pointer connectionStatusItem; private volatile String statusText = null; + @Override + protected + void dispatch(final Runnable runnable) { + GtkSupport.dispatch(runnable); + } @Override public @@ -221,7 +226,7 @@ class GtkTypeSystemTray extends SystemTray { * Called inside the gdk_threads block */ protected - void onMenuAdded(final Pointer menu) {}; + void onMenuAdded(final Pointer menu) {} protected Pointer getMenu() { diff --git a/src/dorkbox/systemTray/swing/SwingSystemTray.java b/src/dorkbox/systemTray/swing/SwingSystemTray.java index ac83148..cf3c7c3 100644 --- a/src/dorkbox/systemTray/swing/SwingSystemTray.java +++ b/src/dorkbox/systemTray/swing/SwingSystemTray.java @@ -98,12 +98,17 @@ class SwingSystemTray extends dorkbox.systemTray.SystemTray { return this.statusText; } + protected + void dispatch(Runnable runnable) { + SwingUtil.invokeLater(runnable); + } + @Override public void setStatus(final String statusText) { this.statusText = statusText; - SwingUtil.invokeLater(new Runnable() { + dispatch(new Runnable() { @Override public void run() { @@ -131,7 +136,7 @@ class SwingSystemTray extends dorkbox.systemTray.SystemTray { @Override protected void setIcon_(final String iconPath) { - SwingUtil.invokeLater(new Runnable() { + dispatch(new Runnable() { @Override public void run() { @@ -145,7 +150,7 @@ class SwingSystemTray extends dorkbox.systemTray.SystemTray { Image trayImage = new ImageIcon(iconPath).getImage() .getScaledInstance(TRAY_SIZE, TRAY_SIZE, Image.SCALE_SMOOTH); trayImage.flush(); - trayIcon = new TrayIcon(trayImage);; + trayIcon = new TrayIcon(trayImage); // appindicators don't support this, so we cater to the lowest common denominator // trayIcon.setToolTip(SwingSystemTray.this.appName); @@ -215,7 +220,7 @@ class SwingSystemTray extends dorkbox.systemTray.SystemTray { throw new NullPointerException("Menu text cannot be null"); } - SwingUtil.invokeLater(new Runnable() { + dispatch(new Runnable() { @Override public void run() {