Changed synchronized -> volatile, changed how synchronized works for

menuEntries Changed setState -> setChecked for checkbox entry
This commit is contained in:
nathan 2016-12-16 22:45:27 +01:00
parent d104c7e510
commit b3b4300179
7 changed files with 153 additions and 103 deletions

View File

@ -26,12 +26,12 @@ import dorkbox.util.SwingUtil;
@SuppressWarnings("unused") @SuppressWarnings("unused")
public public
class Checkbox extends Entry { class Checkbox extends Entry {
private boolean isChecked = false; private volatile boolean isChecked = false;
private String text; private volatile String text;
private ActionListener callback; private volatile ActionListener callback;
private boolean enabled = true; private volatile boolean enabled = true;
private char mnemonicKey; private volatile char mnemonicKey;
public public
Checkbox() { Checkbox() {
@ -54,7 +54,7 @@ class Checkbox extends Entry {
* @param parent the parent of this menu, null if the parent is the system tray * @param parent the parent of this menu, null if the parent is the system tray
* @param systemTray the system tray (which is the object that sits in the system tray) * @param systemTray the system tray (which is the object that sits in the system tray)
*/ */
public synchronized public
void bind(final CheckboxPeer peer, final Menu parent, final SystemTray systemTray) { void bind(final CheckboxPeer peer, final Menu parent, final SystemTray systemTray) {
super.bind(peer, parent, systemTray); super.bind(peer, parent, systemTray);
@ -68,7 +68,7 @@ class Checkbox extends Entry {
/** /**
* @return true if this checkbox is selected, false if not. * @return true if this checkbox is selected, false if not.
*/ */
public synchronized public
boolean getChecked() { boolean getChecked() {
return isChecked; return isChecked;
} }
@ -78,8 +78,8 @@ class Checkbox extends Entry {
* *
* @param isChecked true to show the checkbox, false to hide it * @param isChecked true to show the checkbox, false to hide it
*/ */
public synchronized public
void setState(boolean isChecked) { void setChecked(boolean isChecked) {
this.isChecked = isChecked; this.isChecked = isChecked;
if (peer != null) { if (peer != null) {
@ -90,7 +90,7 @@ class Checkbox extends Entry {
/** /**
* Gets the callback assigned to this menu entry * Gets the callback assigned to this menu entry
*/ */
public synchronized public
ActionListener getCallback() { ActionListener getCallback() {
return callback; return callback;
} }
@ -100,7 +100,7 @@ class Checkbox extends Entry {
* *
* @param callback the callback to set. If null, the callback is safely removed. * @param callback the callback to set. If null, the callback is safely removed.
*/ */
public synchronized public
void setCallback(final ActionListener callback) { void setCallback(final ActionListener callback) {
this.callback = callback; this.callback = callback;
if (peer != null) { if (peer != null) {
@ -111,7 +111,7 @@ class Checkbox extends Entry {
/** /**
* @return true if this item is enabled, or false if it is disabled. * @return true if this item is enabled, or false if it is disabled.
*/ */
public synchronized public
boolean getEnabled() { boolean getEnabled() {
return this.enabled; return this.enabled;
} }
@ -119,7 +119,7 @@ class Checkbox extends Entry {
/** /**
* Enables, or disables the entry. * Enables, or disables the entry.
*/ */
public synchronized public
void setEnabled(final boolean enabled) { void setEnabled(final boolean enabled) {
this.enabled = enabled; this.enabled = enabled;
@ -131,7 +131,7 @@ class Checkbox extends Entry {
/** /**
* @return the text label that the menu entry has assigned * @return the text label that the menu entry has assigned
*/ */
public synchronized public
String getText() { String getText() {
return text; return text;
} }
@ -141,7 +141,7 @@ class Checkbox extends Entry {
* *
* @param text the new text to set * @param text the new text to set
*/ */
public synchronized public
void setText(final String text) { void setText(final String text) {
this.text = text; this.text = text;
@ -157,7 +157,7 @@ class Checkbox extends Entry {
* Mnemonics are case-insensitive, and if the character defined by the mnemonic is found within the text, the first occurrence * Mnemonics are case-insensitive, and if the character defined by the mnemonic is found within the text, the first occurrence
* of it will be underlined. * of it will be underlined.
*/ */
public synchronized public
char getShortcut() { char getShortcut() {
return this.mnemonicKey; return this.mnemonicKey;
} }
@ -170,7 +170,7 @@ class Checkbox extends Entry {
* *
* @param key this is the key to set as the mnemonic * @param key this is the key to set as the mnemonic
*/ */
public synchronized public
void setShortcut(final char key) { void setShortcut(final char key) {
this.mnemonicKey = key; this.mnemonicKey = key;
@ -187,7 +187,7 @@ class Checkbox extends Entry {
* *
* @param key this is the VK key to set as the mnemonic * @param key this is the VK key to set as the mnemonic
*/ */
public synchronized public
void setShortcut(final int key) { void setShortcut(final int key) {
this.mnemonicKey = SwingUtil.getFromVirtualKey(key); this.mnemonicKey = SwingUtil.getFromVirtualKey(key);

View File

@ -28,8 +28,8 @@ class Entry {
private static final AtomicInteger MENU_ID_COUNTER = new AtomicInteger(0); private static final AtomicInteger MENU_ID_COUNTER = new AtomicInteger(0);
private final int id = Entry.MENU_ID_COUNTER.getAndIncrement(); private final int id = Entry.MENU_ID_COUNTER.getAndIncrement();
private Menu parent; private volatile Menu parent;
private SystemTray systemTray; private volatile SystemTray systemTray;
protected volatile EntryPeer peer; protected volatile EntryPeer peer;
@ -45,7 +45,7 @@ class Entry {
* @param parent the parent of this menu, null if the parent is the system tray * @param parent the parent of this menu, null if the parent is the system tray
* @param systemTray the system tray (which is the object that sits in the system tray) * @param systemTray the system tray (which is the object that sits in the system tray)
*/ */
public synchronized public
void bind(final EntryPeer peer, final Menu parent, final SystemTray systemTray) { void bind(final EntryPeer peer, final Menu parent, final SystemTray systemTray) {
this.parent = parent; this.parent = parent;
this.systemTray = systemTray; this.systemTray = systemTray;
@ -59,7 +59,7 @@ class Entry {
/** /**
* @return the parent menu (of this entry or menu) or null if we are the root menu * @return the parent menu (of this entry or menu) or null if we are the root menu
*/ */
public final synchronized public final
Menu getParent() { Menu getParent() {
return this.parent; return this.parent;
} }
@ -67,7 +67,7 @@ class Entry {
/** /**
* @return the system tray that this menu is ultimately attached to * @return the system tray that this menu is ultimately attached to
*/ */
public final synchronized public final
SystemTray getSystemTray() { SystemTray getSystemTray() {
return this.systemTray; return this.systemTray;
} }
@ -75,7 +75,7 @@ class Entry {
/** /**
* Removes this menu entry from the menu and releases all system resources associated with this menu entry * Removes this menu entry from the menu and releases all system resources associated with this menu entry
*/ */
public synchronized public
void remove() { void remove() {
if (peer != null) { if (peer != null) {
peer.remove(); peer.remove();

View File

@ -41,6 +41,7 @@ import dorkbox.systemTray.peer.MenuPeer;
@SuppressWarnings("unused") @SuppressWarnings("unused")
public public
class Menu extends MenuItem { class Menu extends MenuItem {
// access on this object must be synchronized for object visibility
final List<Entry> menuEntries = new ArrayList<Entry>(); final List<Entry> menuEntries = new ArrayList<Entry>();
public public
@ -112,12 +113,20 @@ class Menu extends MenuItem {
* @param parent the parent of this menu, null if the parent is the system tray * @param parent the parent of this menu, null if the parent is the system tray
* @param systemTray the system tray (which is the object that sits in the system tray) * @param systemTray the system tray (which is the object that sits in the system tray)
*/ */
public synchronized public
void bind(final MenuPeer peer, final Menu parent, final SystemTray systemTray) { void bind(final MenuPeer peer, final Menu parent, final SystemTray systemTray) {
super.bind(peer, parent, systemTray); super.bind(peer, parent, systemTray);
for (int i = 0, menuEntriesSize = menuEntries.size(); i < menuEntriesSize; i++) { List<Entry> copy;
final Entry menuEntry = menuEntries.get(i); synchronized (menuEntries) {
// access on this object must be synchronized for object visibility
// a copy is made to prevent deadlocks from occurring when operating in different threads
copy = new ArrayList<Entry>(menuEntries);
}
for (int i = 0, menuEntriesSize = copy.size(); i < menuEntriesSize; i++) {
final Entry menuEntry = copy.get(i);
peer.add(this, menuEntry, i); peer.add(this, menuEntry, i);
} }
} }
@ -204,7 +213,7 @@ class Menu extends MenuItem {
} }
checkbox.setEnabled(entry.isEnabled()); checkbox.setEnabled(entry.isEnabled());
checkbox.setState(entry.getState()); checkbox.setChecked(entry.getState());
checkbox.setShortcut(entry.getMnemonic()); checkbox.setShortcut(entry.getMnemonic());
checkbox.setText(entry.getText()); checkbox.setText(entry.getText());
@ -265,16 +274,19 @@ class Menu extends MenuItem {
/** /**
* Adds a menu entry, separator, or sub-menu to this menu. * Adds a menu entry, separator, or sub-menu to this menu.
*/ */
public synchronized public
<T extends Entry> T add(final T entry, int index) { <T extends Entry> T add(final T entry, int index) {
if (index == -1) { synchronized (menuEntries) {
menuEntries.add(entry); // access on this object must be synchronized for object visibility
} else { if (index == -1) {
if (!menuEntries.isEmpty() && menuEntries.get(0) instanceof Status) { menuEntries.add(entry);
// the "status" menu entry is ALWAYS first } else {
index++; if (!menuEntries.isEmpty() && menuEntries.get(0) instanceof Status) {
// the "status" menu entry is ALWAYS first
index++;
}
menuEntries.add(index, entry);
} }
menuEntries.add(index, entry);
} }
if (peer != null) { if (peer != null) {
@ -295,16 +307,18 @@ class Menu extends MenuItem {
/** /**
* Gets the last menu entry or sub-menu, ignoring status and separators * Gets the last menu entry or sub-menu, ignoring status and separators
*/ */
public synchronized public
Entry getLast() { Entry getLast() {
// Must be wrapped in a synchronized block for object visibility synchronized (menuEntries) {
if (!menuEntries.isEmpty()) { // access on this object must be synchronized for object visibility
Entry entry; if (!menuEntries.isEmpty()) {
for (int i = menuEntries.size()-1; i >= 0; i--) { Entry entry;
entry = menuEntries.get(i); for (int i = menuEntries.size() - 1; i >= 0; i--) {
entry = menuEntries.get(i);
if (!(entry instanceof Separator || entry instanceof Status)) { if (!(entry instanceof Separator || entry instanceof Status)) {
return entry; return entry;
}
} }
} }
} }
@ -317,25 +331,27 @@ class Menu extends MenuItem {
* *
* @param menuIndex the menu entry index to use to retrieve the menu entry. * @param menuIndex the menu entry index to use to retrieve the menu entry.
*/ */
public synchronized public
Entry get(final int menuIndex) { Entry get(final int menuIndex) {
if (menuIndex < 0) { if (menuIndex < 0) {
return null; return null;
} }
// Must be wrapped in a synchronized block for object visibility synchronized (menuEntries) {
if (!menuEntries.isEmpty()) { // access on this object must be synchronized for object visibility
int count = 0; if (!menuEntries.isEmpty()) {
for (Entry entry : menuEntries) { int count = 0;
if (entry instanceof Separator || entry instanceof Status) { for (Entry entry : menuEntries) {
continue; if (entry instanceof Separator || entry instanceof Status) {
} continue;
}
if (count == menuIndex) { if (count == menuIndex) {
return entry; return entry;
} }
count++; count++;
}
} }
} }
@ -347,46 +363,77 @@ class Menu extends MenuItem {
* *
* @param entry This is the menu entry to remove * @param entry This is the menu entry to remove
*/ */
public synchronized public
void remove(final Entry entry) { void remove(final Entry entry) {
// null is passed in when a sub-menu is removing itself from us (because they have already called "remove" and have also // null is passed in when a sub-menu is removing itself from us (because they have already called "remove" and have also
// removed themselves from the menuEntries) // removed themselves from the menuEntries)
if (entry != null) { if (entry != null) {
for (Iterator<Entry> iterator = menuEntries.iterator(); iterator.hasNext(); ) { Entry toRemove = null;
final Entry entry__ = iterator.next();
if (entry__ == entry) { synchronized (menuEntries) {
iterator.remove(); // access on this object must be synchronized for object visibility
entry.remove(); for (Iterator<Entry> iterator = menuEntries.iterator(); iterator.hasNext(); ) {
break; final Entry entry__ = iterator.next();
if (entry__ == entry) {
iterator.remove();
toRemove = entry__;
break;
}
} }
} }
if (toRemove != null) {
toRemove.remove();
toRemove = null;
}
// now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function.
if (!menuEntries.isEmpty()) { synchronized (menuEntries) {
if (menuEntries.get(0) instanceof dorkbox.systemTray.Separator) { // access on this object must be synchronized for object visibility
remove(menuEntries.get(0)); if (!menuEntries.isEmpty()) {
if (menuEntries.get(0) instanceof dorkbox.systemTray.Separator) {
toRemove = menuEntries.get(0);
}
} }
} }
if (toRemove != null) {
remove(toRemove);
toRemove = null;
}
// now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function. // now check to see if a spacer is at the top/bottom of the list (and remove it if so. This is a recursive function.
if (!menuEntries.isEmpty()) { synchronized (menuEntries) {
if (menuEntries.get(menuEntries.size()-1) instanceof dorkbox.systemTray.Separator) { // access on this object must be synchronized for object visibility
remove(menuEntries.get(menuEntries.size() - 1)); if (!menuEntries.isEmpty()) {
if (menuEntries.get(menuEntries.size()-1) instanceof dorkbox.systemTray.Separator) {
toRemove = menuEntries.get(menuEntries.size() - 1);
}
} }
} }
if (toRemove != null) {
remove(toRemove);
}
} }
} }
/** /**
* This removes all menu entries from this menu * This removes all menu entries from this menu
*/ */
public synchronized public
void clear() { void clear() {
// have to make copy because we are deleting all of them, and sub-menus remove themselves from parents List<Entry> copy;
ArrayList<Entry> menuEntriesCopy = new ArrayList<Entry>(this.menuEntries); synchronized (menuEntries) {
for (Entry entry : menuEntriesCopy) { // access on this object must be synchronized for object visibility
// a copy is made to prevent deadlocks from occurring when operating in different threads
// have to make copy because we are deleting all of them, and sub-menus remove themselves from parents
copy = new ArrayList<Entry>(menuEntries);
menuEntries.clear();
}
for (Entry entry : copy) {
entry.remove(); entry.remove();
} }
menuEntries.clear();
} }
@ -394,7 +441,7 @@ class Menu extends MenuItem {
* This removes all menu entries from this menu AND this menu from it's parent * This removes all menu entries from this menu AND this menu from it's parent
*/ */
@Override @Override
public synchronized public
void remove() { void remove() {
clear(); clear();

View File

@ -33,13 +33,13 @@ import dorkbox.util.SwingUtil;
@SuppressWarnings({"unused", "SameParameterValue", "WeakerAccess"}) @SuppressWarnings({"unused", "SameParameterValue", "WeakerAccess"})
public public
class MenuItem extends Entry { class MenuItem extends Entry {
private String text; private volatile String text;
private File imageFile; private volatile File imageFile;
private ActionListener callback; private volatile ActionListener callback;
// default enabled is always true // default enabled is always true
private boolean enabled = true; private volatile boolean enabled = true;
private char mnemonicKey; private volatile char mnemonicKey;
public public
MenuItem() { MenuItem() {
@ -124,7 +124,7 @@ class MenuItem extends Entry {
* @param parent the parent of this menu, null if the parent is the system tray * @param parent the parent of this menu, null if the parent is the system tray
* @param systemTray the system tray (which is the object that sits in the system tray) * @param systemTray the system tray (which is the object that sits in the system tray)
*/ */
public synchronized public
void bind(final MenuItemPeer peer, final Menu parent, final SystemTray systemTray) { void bind(final MenuItemPeer peer, final Menu parent, final SystemTray systemTray) {
super.bind(peer, parent, systemTray); super.bind(peer, parent, systemTray);
@ -135,7 +135,7 @@ class MenuItem extends Entry {
peer.setShortcut(this); peer.setShortcut(this);
} }
private synchronized private
void setImage_(final File imageFile) { void setImage_(final File imageFile) {
this.imageFile = imageFile; this.imageFile = imageFile;
@ -149,7 +149,7 @@ class MenuItem extends Entry {
* <p> * <p>
* This file can also be a cached file, depending on how the image was assigned to this entry. * This file can also be a cached file, depending on how the image was assigned to this entry.
*/ */
public synchronized public
File getImage() { File getImage() {
return imageFile; return imageFile;
} }
@ -157,7 +157,7 @@ class MenuItem extends Entry {
/** /**
* Gets the callback assigned to this menu entry * Gets the callback assigned to this menu entry
*/ */
public synchronized public
ActionListener getCallback() { ActionListener getCallback() {
return callback; return callback;
} }
@ -165,7 +165,7 @@ class MenuItem extends Entry {
/** /**
* @return true if this item is enabled, or false if it is disabled. * @return true if this item is enabled, or false if it is disabled.
*/ */
public synchronized public
boolean getEnabled() { boolean getEnabled() {
return this.enabled; return this.enabled;
} }
@ -173,7 +173,7 @@ class MenuItem extends Entry {
/** /**
* Enables, or disables the entry. * Enables, or disables the entry.
*/ */
public synchronized public
void setEnabled(final boolean enabled) { void setEnabled(final boolean enabled) {
this.enabled = enabled; this.enabled = enabled;
@ -185,7 +185,7 @@ class MenuItem extends Entry {
/** /**
* @return the text label that the menu entry has assigned * @return the text label that the menu entry has assigned
*/ */
public synchronized public
String getText() { String getText() {
return text; return text;
} }
@ -195,7 +195,7 @@ class MenuItem extends Entry {
* *
* @param text the new text to set * @param text the new text to set
*/ */
public synchronized public
void setText(final String text) { void setText(final String text) {
this.text = text; this.text = text;
@ -280,7 +280,7 @@ class MenuItem extends Entry {
/** /**
* @return true if this menu entry has an image assigned to it, or is just text. * @return true if this menu entry has an image assigned to it, or is just text.
*/ */
public synchronized public
boolean hasImage() {return imageFile != null;} boolean hasImage() {return imageFile != null;}
/** /**
@ -288,7 +288,7 @@ class MenuItem extends Entry {
* *
* @param callback the callback to set. If null, the callback is safely removed. * @param callback the callback to set. If null, the callback is safely removed.
*/ */
public synchronized public
void setCallback(final ActionListener callback) { void setCallback(final ActionListener callback) {
this.callback = callback; this.callback = callback;
@ -304,7 +304,7 @@ class MenuItem extends Entry {
* Mnemonics are case-insensitive, and if the character defined by the mnemonic is found within the text, the first occurrence * Mnemonics are case-insensitive, and if the character defined by the mnemonic is found within the text, the first occurrence
* of it will be underlined. * of it will be underlined.
*/ */
public synchronized public
char getShortcut() { char getShortcut() {
return this.mnemonicKey; return this.mnemonicKey;
} }
@ -317,7 +317,7 @@ class MenuItem extends Entry {
* *
* @param key this is the key to set as the mnemonic * @param key this is the key to set as the mnemonic
*/ */
public synchronized public
void setShortcut(final char key) { void setShortcut(final char key) {
this.mnemonicKey = key; this.mnemonicKey = key;
@ -334,7 +334,7 @@ class MenuItem extends Entry {
* *
* @param key this is the VK key to set as the mnemonic * @param key this is the VK key to set as the mnemonic
*/ */
public synchronized public
void setShortcut(final int key) { void setShortcut(final int key) {
this.mnemonicKey = SwingUtil.getFromVirtualKey(key); this.mnemonicKey = SwingUtil.getFromVirtualKey(key);
@ -344,7 +344,7 @@ class MenuItem extends Entry {
} }
@Override @Override
public synchronized public
void remove() { void remove() {
if (peer != null) { if (peer != null) {
setImage_(null); setImage_(null);

View File

@ -33,7 +33,7 @@ class Status extends Entry {
* @param parent the parent of this menu, null if the parent is the system tray * @param parent the parent of this menu, null if the parent is the system tray
* @param systemTray the system tray (which is the object that sits in the system tray) * @param systemTray the system tray (which is the object that sits in the system tray)
*/ */
public synchronized public
void bind(final StatusPeer peer, final Menu parent, final SystemTray systemTray) { void bind(final StatusPeer peer, final Menu parent, final SystemTray systemTray) {
super.bind(peer, parent, systemTray); super.bind(peer, parent, systemTray);
@ -43,7 +43,7 @@ class Status extends Entry {
/** /**
* @return the text label that the menu entry has assigned * @return the text label that the menu entry has assigned
*/ */
public synchronized public
String getText() { String getText() {
return text; return text;
} }
@ -53,7 +53,7 @@ class Status extends Entry {
* *
* @param text the new text to set * @param text the new text to set
*/ */
public synchronized public
void setText(final String text) { void setText(final String text) {
this.text = text; this.text = text;

View File

@ -20,7 +20,6 @@ class Tray extends Menu {
// appindicators DO NOT support anything other than PLAIN gtk-menus (which we hack to support swing menus) // appindicators DO NOT support anything other than PLAIN gtk-menus (which we hack to support swing menus)
// they ALSO do not support tooltips, so we cater to the lowest common denominator // they ALSO do not support tooltips, so we cater to the lowest common denominator
// trayIcon.setToolTip("app name");
private volatile String statusText; private volatile String statusText;
@ -32,7 +31,7 @@ class Tray extends Menu {
/** /**
* Gets the 'status' string assigned to the system tray * Gets the 'status' string assigned to the system tray
*/ */
public synchronized public
String getStatus() { String getStatus() {
return statusText; return statusText;
} }
@ -42,14 +41,18 @@ class Tray extends Menu {
* *
* @param statusText the text you want displayed, null if you want to remove the 'status' string * @param statusText the text you want displayed, null if you want to remove the 'status' string
*/ */
public synchronized public
void setStatus(final String statusText) { void setStatus(final String statusText) {
this.statusText = statusText; this.statusText = statusText;
// status is ALWAYS at 0 index... // status is ALWAYS at 0 index...
Entry menuEntry = null; Entry menuEntry = null;
if (!menuEntries.isEmpty()) {
menuEntry = menuEntries.get(0); synchronized (menuEntries) {
// access on this object must be synchronized for object visibility
if (!menuEntries.isEmpty()) {
menuEntry = menuEntries.get(0);
}
} }
if (menuEntry instanceof Status) { if (menuEntry instanceof Status) {

View File

@ -91,7 +91,7 @@ class SwingMenuItemCheckbox implements CheckboxPeer {
public public
void actionPerformed(ActionEvent e) { void actionPerformed(ActionEvent e) {
// this will run on the EDT, since we are calling it from the EDT // this will run on the EDT, since we are calling it from the EDT
menuItem.setState(!isChecked); menuItem.setChecked(!isChecked);
// we want it to run on the EDT, but with our own action event info (so it is consistent across all platforms) // we want it to run on the EDT, but with our own action event info (so it is consistent across all platforms)
ActionListener cb = menuItem.getCallback(); ActionListener cb = menuItem.getCallback();