From 091962ec9dbc8988f6dfeaaedea62d7bc889bf6c Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sat, 30 Nov 2024 01:02:47 -0600 Subject: [PATCH] Use weak references for listener lists, closes #85 --- CHANGELOG.md | 8 +- CONTRIBUTING.md | 8 +- .../org/deepsymmetry/beatlink/BeatFinder.java | 133 +++++++-------- .../deepsymmetry/beatlink/DeviceFinder.java | 24 ++- .../beatlink/LifecycleParticipant.java | 25 ++- .../java/org/deepsymmetry/beatlink/Util.java | 68 +++++++- .../org/deepsymmetry/beatlink/VirtualCdj.java | 7 +- .../beatlink/data/AnalysisTagFinder.java | 28 ++-- .../deepsymmetry/beatlink/data/ArtFinder.java | 28 ++-- .../beatlink/data/BeatGridFinder.java | 27 ++-- .../beatlink/data/CrateDigger.java | 28 ++-- .../beatlink/data/MetadataFinder.java | 47 +++--- .../beatlink/data/SignatureFinder.java | 22 ++- .../beatlink/data/TimeFinder.java | 152 ++++++++++++------ .../beatlink/data/WaveformFinder.java | 22 ++- 15 files changed, 365 insertions(+), 262 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66f2f56..4b7c2ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ This change log follows the conventions of - An error in interpreting database export file format by the Crate Digger library could lead to some rows that were actually present in tables not being found. +### Changed + +- Whenever listeners are registered for events produced by Beat Link classes, they are now wrapped in `WeakReference` containers, so that the listeners can be garbage collected even if they have been registered. Beat Link will notice that the `WeakReference` content has become `null`, and remove it from the listener list. + ## [7.4.0] - 2024-05-04 May the Fourth be with you. @@ -744,8 +748,8 @@ May the Fourth be with you. ### Changed -- Device updates, beat announcements, and master announcements are time - sensitive, so they are now delivered directly on the thread that is +- Device updates, beat announcements, and master announcements are + time-sensitive, so they are now delivered directly on the thread that is receiving them from the network, rather than being added to the Event Dispatch Queue. This will reduce latency, but means listener methods need to be very fast, and delegate any lengthy, non-time-sensitive diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d75a7fc..0c00dc3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,7 +8,7 @@ great. First of all, we would *love* to hear from you! We have no way of knowing who has discovered, explored, downloaded and tried Beat Link. So if you have, please write a quick note on the [Beat Link Trigger Zulip -stream](https://deep-symmetry.zulipchat.com/#narrow/stream/275322-beat-link-trigger) +channel](https://deep-symmetry.zulipchat.com/#narrow/stream/275322-beat-link-trigger) to let us know! Even if it is only to explain why it didn’t quite work for you. @@ -44,8 +44,10 @@ Once you have something working you’d like to share, you can open a [pull request][pulls]. Or if you simply have an idea, or something that you wish worked -differently, feel free to open an [issue][issues] if it seems like -nobody already has. +differently, feel free to discuss it on the [Beat Link Trigger Zulip +channel](https://deep-symmetry.zulipchat.com/#narrow/stream/275322-beat-link-trigger), +and if directed to do so by the community there, open an +[issue][issues]. ## Maintainers diff --git a/src/main/java/org/deepsymmetry/beatlink/BeatFinder.java b/src/main/java/org/deepsymmetry/beatlink/BeatFinder.java index 546d28e..a2247b4 100644 --- a/src/main/java/org/deepsymmetry/beatlink/BeatFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/BeatFinder.java @@ -6,11 +6,11 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.lang.ref.WeakReference; import java.net.DatagramPacket; import java.net.DatagramSocket; import java.net.SocketException; import java.util.*; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; /** @@ -279,13 +279,13 @@ public synchronized void stop() { /** * Keeps track of the registered beat listeners, except the TimeFinder's. */ - private final Set beatListeners = - Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> beatListeners = new LinkedList<>(); /** *

Adds the specified beat listener to receive beat announcements when DJ Link devices broadcast * them on the network. If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed.

+ * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, beat announcements are delivered to listeners directly on the thread that is receiving them * them from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -299,11 +299,11 @@ public synchronized void stop() { * @param listener the beat listener to add */ @API(status = API.Status.STABLE) - public void addBeatListener(BeatListener listener) { + public synchronized void addBeatListener(BeatListener listener) { if (TimeFinder.getInstance().isOwnBeatListener(listener)) { timeFinderBeatListener.set(listener); - } else if (listener != null) { - beatListeners.add(listener); + } else { + Util.addListener(beatListeners, listener); } } @@ -315,11 +315,11 @@ public void addBeatListener(BeatListener listener) { * @param listener the beat listener to remove */ @API(status = API.Status.STABLE) - public void removeBeatListener(BeatListener listener) { + public synchronized void removeBeatListener(BeatListener listener) { if (TimeFinder.getInstance().isOwnBeatListener(listener)) { timeFinderBeatListener.set(null); - } else if (listener != null) { - beatListeners.remove(listener); + } else { + Util.removeListener(beatListeners, listener); } } @@ -329,9 +329,9 @@ public void removeBeatListener(BeatListener listener) { * @return the currently registered beat listeners */ @API(status = API.Status.STABLE) - public Set getBeatListeners() { + public synchronized Set getBeatListeners() { // Make a copy so callers get an immutable snapshot of the current state. - final Set result = new HashSet<>(beatListeners); + final Set result = Util.gatherListeners(beatListeners); final BeatListener timeFinderListener = timeFinderBeatListener.get(); if (timeFinderListener != null) { result.add(timeFinderListener); @@ -355,7 +355,7 @@ private void deliverBeat(final Beat beat) { logger.warn("Problem delivering beat announcement to TimeFinder listener", t); } } - for (final BeatListener listener : new LinkedList<>(beatListeners)) { + for (final BeatListener listener : Util.gatherListeners(beatListeners)) { try { listener.newBeat(beat); } catch (Throwable t) { @@ -367,12 +367,13 @@ private void deliverBeat(final Beat beat) { /** * Keeps track of the registered precise position listeners. */ - private final Set precisePositionListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> precisePositionListeners = new LinkedList<>(); /** *

Adds the specified precise position listener to receive precise position updates when DJ Link devices send * them to Beat Link. If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed.

+ * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, precise position updates are delivered to listeners directly on the thread that is receiving them * them from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -387,10 +388,8 @@ private void deliverBeat(final Beat beat) { * @param listener the precise position listener to add */ @API(status = API.Status.STABLE) - public void addPrecisePositionListener(PrecisePositionListener listener) { - if (listener != null) { - precisePositionListeners.add(listener); - } + public synchronized void addPrecisePositionListener(PrecisePositionListener listener) { + Util.addListener(precisePositionListeners, listener); } /** @@ -401,10 +400,8 @@ public void addPrecisePositionListener(PrecisePositionListener listener) { * @param listener the precise position listener to remove */ @API(status = API.Status.STABLE) - public void removePrecisePositionListener(PrecisePositionListener listener) { - if (listener != null) { - precisePositionListeners.remove(listener); - } + public synchronized void removePrecisePositionListener(PrecisePositionListener listener) { + Util.removeListener(precisePositionListeners, listener); } /** @@ -413,9 +410,9 @@ public void removePrecisePositionListener(PrecisePositionListener listener) { * @return the currently registered precise position listeners */ @API(status = API.Status.STABLE) - public Set getPrecisePositionListeners() { + public synchronized Set getPrecisePositionListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(precisePositionListeners); + return Collections.unmodifiableSet(Util.gatherListeners(precisePositionListeners)); } /** @@ -436,12 +433,13 @@ private void deliverPrecisePosition(PrecisePosition position) { /** * Keeps track of the registered sync command listeners. */ - private final Set syncListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> syncListeners = new LinkedList<>(); /** *

Adds the specified sync command listener to receive sync commands when DJ Link devices send * them to Beat Link. If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed.

+ * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, sync commands are delivered to listeners directly on the thread that is receiving them * them from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -455,10 +453,8 @@ private void deliverPrecisePosition(PrecisePosition position) { * @param listener the sync listener to add */ @API(status = API.Status.STABLE) - public void addSyncListener(SyncListener listener) { - if (listener != null) { - syncListeners.add(listener); - } + public synchronized void addSyncListener(SyncListener listener) { + Util.addListener(syncListeners, listener); } /** @@ -469,10 +465,8 @@ public void addSyncListener(SyncListener listener) { * @param listener the sync listener to remove */ @API(status = API.Status.STABLE) - public void removeSyncListener(SyncListener listener) { - if (listener != null) { - syncListeners.remove(listener); - } + public synchronized void removeSyncListener(SyncListener listener) { + Util.removeListener(syncListeners, listener); } /** @@ -481,9 +475,9 @@ public void removeSyncListener(SyncListener listener) { * @return the currently registered sync listeners */ @API(status = API.Status.STABLE) - public Set getSyncListeners() { + public synchronized Set getSyncListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(syncListeners); + return Collections.unmodifiableSet(Util.gatherListeners(syncListeners)); } /** @@ -515,14 +509,15 @@ private void deliverSyncCommand(byte command) { } /** - * Keeps track of the registered master handoff command listeners. + * Keeps track of the registered master handoff command listeners. */ - private final Set masterHandoffListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> masterHandoffListeners = new LinkedList<>(); /** *

Adds the specified master handoff listener to receive tempo master handoff commands when DJ Link devices send * them to Beat Link. If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed.

+ * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, handoff commands are delivered to listeners directly on the thread that is receiving them * them from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -536,10 +531,8 @@ private void deliverSyncCommand(byte command) { * @param listener the tempo master handoff listener to add */ @API(status = API.Status.STABLE) - public void addMasterHandoffListener(MasterHandoffListener listener) { - if (listener != null) { - masterHandoffListeners.add(listener); - } + public synchronized void addMasterHandoffListener(MasterHandoffListener listener) { + Util.addListener(masterHandoffListeners, listener); } /** @@ -550,10 +543,8 @@ public void addMasterHandoffListener(MasterHandoffListener listener) { * @param listener the tempo master handoff listener to remove */ @API(status = API.Status.STABLE) - public void removeMasterHandoffListener(MasterHandoffListener listener) { - if (listener != null) { - masterHandoffListeners.remove(listener); - } + public synchronized void removeMasterHandoffListener(MasterHandoffListener listener) { + Util.removeListener(masterHandoffListeners, listener); } /** @@ -562,9 +553,9 @@ public void removeMasterHandoffListener(MasterHandoffListener listener) { * @return the currently registered tempo master handoff command listeners */ @API(status = API.Status.STABLE) - public Set getMasterHandoffListeners() { + public synchronized Set getMasterHandoffListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(masterHandoffListeners); + return Collections.unmodifiableSet(Util.gatherListeners(masterHandoffListeners)); } /** @@ -601,12 +592,13 @@ private void deliverMasterYieldResponse(int fromPlayer, boolean yielded) { /** * Keeps track of the registered on-air listeners. */ - private final Set onAirListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> onAirListeners = new LinkedList<>(); /** *

Adds the specified on-air listener to receive channel on-air updates when the mixer broadcasts * them on the network. If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed.

+ * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, on-air updates are delivered to listeners directly on the thread that is receiving them * them from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -620,10 +612,8 @@ private void deliverMasterYieldResponse(int fromPlayer, boolean yielded) { * @param listener the on-air listener to add */ @API(status = API.Status.STABLE) - public void addOnAirListener(OnAirListener listener) { - if (listener != null) { - onAirListeners.add(listener); - } + public synchronized void addOnAirListener(OnAirListener listener) { + Util.addListener(onAirListeners, listener); } /** @@ -634,10 +624,8 @@ public void addOnAirListener(OnAirListener listener) { * @param listener the on-air listener to remove */ @API(status = API.Status.STABLE) - public void removeOnAirListener(OnAirListener listener) { - if (listener != null) { - onAirListeners.remove(listener); - } + public synchronized void removeOnAirListener(OnAirListener listener) { + Util.removeListener(onAirListeners, listener); } /** @@ -646,9 +634,9 @@ public void removeOnAirListener(OnAirListener listener) { * @return the currently registered on-air listeners */ @API(status = API.Status.STABLE) - public Set getOnAirListeners() { + public synchronized Set getOnAirListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(onAirListeners); + return Collections.unmodifiableSet(Util.gatherListeners(onAirListeners)); } /** @@ -669,12 +657,13 @@ private void deliverOnAirUpdate(Set audibleChannels) { /** * Keeps track of the registered fader start listeners. */ - private final Set faderStartListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> faderStartListeners = new LinkedList<>(); /** *

Adds the specified fader start listener to receive fader start commands when the mixer broadcasts * them on the network. If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed.

+ * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, fader start commands are delivered to listeners directly on the thread that is receiving them * them from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -688,10 +677,8 @@ private void deliverOnAirUpdate(Set audibleChannels) { * @param listener the fader start listener to add */ @API(status = API.Status.STABLE) - public void addFaderStartListener(FaderStartListener listener) { - if (listener != null) { - faderStartListeners.add(listener); - } + public synchronized void addFaderStartListener(FaderStartListener listener) { + Util.addListener(faderStartListeners, listener); } /** @@ -702,10 +689,8 @@ public void addFaderStartListener(FaderStartListener listener) { * @param listener the fader start listener to remove */ @API(status = API.Status.STABLE) - public void removeFaderStartListener(FaderStartListener listener) { - if (listener != null) { - faderStartListeners.remove(listener); - } + public synchronized void removeFaderStartListener(FaderStartListener listener) { + Util.removeListener(faderStartListeners, listener); } /** @@ -714,9 +699,9 @@ public void removeFaderStartListener(FaderStartListener listener) { * @return the currently registered fader start listeners */ @API(status = API.Status.STABLE) - public Set getFaderStartListeners() { + public synchronized Set getFaderStartListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(faderStartListeners); + return Collections.unmodifiableSet(Util.gatherListeners(faderStartListeners)); } /** diff --git a/src/main/java/org/deepsymmetry/beatlink/DeviceFinder.java b/src/main/java/org/deepsymmetry/beatlink/DeviceFinder.java index dc72ad2..cf526bb 100644 --- a/src/main/java/org/deepsymmetry/beatlink/DeviceFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/DeviceFinder.java @@ -6,6 +6,7 @@ import javax.swing.*; import java.io.IOException; +import java.lang.ref.WeakReference; import java.net.*; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -369,12 +370,13 @@ public DeviceAnnouncement getLatestAnnouncementFrom(int deviceNumber) { /** * Keeps track of the registered device announcement listeners. */ - private final Set deviceListeners = - Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> deviceListeners = new LinkedList<>(); + /** * Adds the specified device announcement listener to receive device announcements when DJ Link devices * are found on or leave the network. If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed. + * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references. * *

Device announcements are delivered to listeners on the * Event Dispatch thread, @@ -384,10 +386,8 @@ public DeviceAnnouncement getLatestAnnouncementFrom(int deviceNumber) { * @param listener the device announcement listener to add */ @API(status = API.Status.STABLE) - public void addDeviceAnnouncementListener(DeviceAnnouncementListener listener) { - if (listener != null) { - deviceListeners.add(listener); - } + public synchronized void addDeviceAnnouncementListener(DeviceAnnouncementListener listener) { + Util.addListener(deviceListeners, listener); } /** @@ -397,10 +397,8 @@ public void addDeviceAnnouncementListener(DeviceAnnouncementListener listener) { * * @param listener the device announcement listener to remove */ - public void removeDeviceAnnouncementListener(DeviceAnnouncementListener listener) { - if (listener != null) { - deviceListeners.remove(listener); - } + public synchronized void removeDeviceAnnouncementListener(DeviceAnnouncementListener listener) { + Util.removeListener(deviceListeners, listener); } /** @@ -409,9 +407,9 @@ public void removeDeviceAnnouncementListener(DeviceAnnouncementListener listener * @return the currently registered device announcement listeners */ @API(status = API.Status.STABLE) - public Set getDeviceAnnouncementListeners() { + public synchronized Set getDeviceAnnouncementListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(deviceListeners); + return Collections.unmodifiableSet(Util.gatherListeners(deviceListeners)); } /** diff --git a/src/main/java/org/deepsymmetry/beatlink/LifecycleParticipant.java b/src/main/java/org/deepsymmetry/beatlink/LifecycleParticipant.java index 4ef26ad..551db43 100644 --- a/src/main/java/org/deepsymmetry/beatlink/LifecycleParticipant.java +++ b/src/main/java/org/deepsymmetry/beatlink/LifecycleParticipant.java @@ -3,9 +3,11 @@ import org.apiguardian.api.API; import org.slf4j.Logger; +import java.lang.ref.WeakReference; import java.util.Collections; +import java.util.LinkedList; +import java.util.List; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; /** * Provides the abstract skeleton for all the classes that can be started and stopped in Beat Link, and for which @@ -17,12 +19,13 @@ public abstract class LifecycleParticipant { /** * Keeps track of the registered device announcement listeners. */ - private final Set lifecycleListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> lifecycleListeners = new LinkedList<>(); /** *

Adds the specified life cycle listener to receive announcements when the component starts and stops. * If {@code listener} is {@code null} or already present in the list - * of registered listeners, no exception is thrown and no action is performed.

+ * of registered listeners, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

Lifecycle announcements are delivered to listeners on a separate thread to avoid worries about deadlock in * synchronized start and stop methods. The called function should still be fast, or delegate long operations to @@ -31,10 +34,8 @@ public abstract class LifecycleParticipant { * @param listener the device announcement listener to add */ @API(status = API.Status.STABLE) - public void addLifecycleListener(LifecycleListener listener) { - if (listener != null) { - lifecycleListeners.add(listener); - } + public synchronized void addLifecycleListener(LifecycleListener listener) { + Util.addListener(lifecycleListeners, listener); } /** @@ -45,10 +46,8 @@ public void addLifecycleListener(LifecycleListener listener) { * @param listener the life cycle listener to remove */ @API(status = API.Status.STABLE) - public void removeLifecycleListener(LifecycleListener listener) { - if (listener != null) { - lifecycleListeners.remove(listener); - } + public synchronized void removeLifecycleListener(LifecycleListener listener) { + Util.removeListener(lifecycleListeners, listener); } /** @@ -57,9 +56,9 @@ public void removeLifecycleListener(LifecycleListener listener) { * @return the currently registered lifecycle listeners */ @API(status = API.Status.STABLE) - public Set getLifecycleListeners() { + public synchronized Set getLifecycleListeners() { // Make a copy so the caller gets an immutable snapshot of the current moment in time. - return Set.copyOf(lifecycleListeners); + return Collections.unmodifiableSet(Util.gatherListeners(lifecycleListeners)); } /** diff --git a/src/main/java/org/deepsymmetry/beatlink/Util.java b/src/main/java/org/deepsymmetry/beatlink/Util.java index ea8e2a8..6412f06 100644 --- a/src/main/java/org/deepsymmetry/beatlink/Util.java +++ b/src/main/java/org/deepsymmetry/beatlink/Util.java @@ -8,6 +8,7 @@ import java.awt.*; import java.io.IOException; +import java.lang.ref.WeakReference; import java.net.DatagramPacket; import java.net.InetAddress; import java.net.InterfaceAddress; @@ -15,6 +16,7 @@ import java.nio.ByteBuffer; import java.nio.channels.WritableByteChannel; import java.util.*; +import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.IntStream; @@ -514,6 +516,71 @@ public static void writeFully(ByteBuffer buffer, WritableByteChannel channel) th } } + /** + * Add a listener to one of the weakly-held listener lists maintained by Beat Link classes. Does nothing + * if the listener is already on the list. Also cleans out any references to garbage-collected listeners. + * Attempts to add a `null` listener are ignored, but will still clean out garbage-collected references. + * + * @param listenerList the listener list to be added to + * @param listener the listener to add + * @param the type of listeners weakly referenced by the list + */ + public static void addListener(List> listenerList, T listener) { + Iterator> iterator = listenerList.iterator(); + while (iterator.hasNext()) { + T currentListener = iterator.next().get(); + + if (currentListener == null) { + iterator.remove(); // We found a garbage-collected listener. + } else if (currentListener == listener) { + return; // The listener was already on the list. + } + } + + if (listener != null) { + listenerList.add(new WeakReference<>(listener)); + } + } + + /** + * Remove a listener from one of the weakly-held listener lists maintained by Beat Link classes. Does nothing + * if the listener is not on the list, except for cleaning out any references to garbage-collected listeners. + * + * @param listenerList the listener list to be removed from + * @param listener the listener to remove + * @param the type of listeners weakly referenced by the list + */ + public static void removeListener(List> listenerList, T listener) { + Iterator> iterator = listenerList.iterator(); + while (iterator.hasNext()) { + T currentListener = iterator.next().get(); + if (currentListener == listener || currentListener == null) iterator.remove(); + } + } + + /** + * Gather a set of the surviving listeners from a weakly-held listener list, for exposure through the API. + * Removes any that have been garbage collected from the listener list. + * + * @param listenerList the listener list whose non-garbage-collected members should be returned + * @param the type of listeners weakly referenced by the list + * + * @return the listeners that were registered and have not been garbage collected + */ + public static Set gatherListeners(List> listenerList) { + Set result = new HashSet<>(); + Iterator> iterator = listenerList.iterator(); + while (iterator.hasNext()) { + T listener = iterator.next().get(); + if (listener == null) { + iterator.remove(); + } else { + result.add(listener); + } + } + return result; + } + /** * Figure out the track time that corresponds to a half-frame number (75 frames per second, so 150 half-frames). * @@ -1074,7 +1141,6 @@ public static int translateOpusPlayerNumbers(int reportedPlayerNumber) { return reportedPlayerNumber & 7; } - /** * Prevent instantiation. */ diff --git a/src/main/java/org/deepsymmetry/beatlink/VirtualCdj.java b/src/main/java/org/deepsymmetry/beatlink/VirtualCdj.java index 990af50..0187127 100644 --- a/src/main/java/org/deepsymmetry/beatlink/VirtualCdj.java +++ b/src/main/java/org/deepsymmetry/beatlink/VirtualCdj.java @@ -28,6 +28,7 @@ * * @author James Elliott */ +@SuppressWarnings("LoggingSimilarMessage") @API(status = API.Status.STABLE) public class VirtualCdj extends LifecycleParticipant { @@ -927,9 +928,9 @@ private boolean createVirtualCdj() throws SocketException { } // Copy the chosen interface's hardware and IP addresses into the announcement packet template - byte[] addr = matchingInterfaces.get(0).getHardwareAddress(); - if (addr != null) { - System.arraycopy(addr, 0, keepAliveBytes, MAC_ADDRESS_OFFSET, 6); + byte[] address = matchingInterfaces.get(0).getHardwareAddress(); + if (address != null) { + System.arraycopy(address, 0, keepAliveBytes, MAC_ADDRESS_OFFSET, 6); } System.arraycopy(matchedAddress.getAddress().getAddress(), 0, keepAliveBytes, 44, 4); broadcastAddress.set(matchedAddress.getBroadcast()); diff --git a/src/main/java/org/deepsymmetry/beatlink/data/AnalysisTagFinder.java b/src/main/java/org/deepsymmetry/beatlink/data/AnalysisTagFinder.java index a415f79..575f2f3 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/AnalysisTagFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/AnalysisTagFinder.java @@ -9,6 +9,7 @@ import org.slf4j.LoggerFactory; import javax.swing.*; +import java.lang.ref.WeakReference; import java.nio.ByteBuffer; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -443,12 +444,13 @@ RekordboxAnlz.TaggedSection getTagViaDbServer(final int rekordboxId, final SlotR /** * Keeps track of the registered tag listeners, indexed by the type of tag they are listening for. */ - private final Map> analysisTagListeners = new ConcurrentHashMap<>(); + private final Map>> analysisTagListeners = new ConcurrentHashMap<>(); /** *

Adds the specified listener to receive updates when track analysis information of a specific type for a player changes. * If {@code listener} is {@code null} or already present in the set of registered listeners for the specified file - * extension and tag type, no exception is thrown and no action is performed.

+ * extension and tag type, no exception is thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

Updates are delivered to listeners on the Swing Event Dispatch thread, so it is safe to interact with * user interface elements within the event handler.

@@ -466,13 +468,13 @@ public synchronized void addAnalysisTagListener(final AnalysisTagListener listen if (listener != null) { final String tagKey = typeTag + fileExtension; boolean trackingNewTag = false; - Set specificTagListeners = analysisTagListeners.get(tagKey); + List> specificTagListeners = analysisTagListeners.get(tagKey); if (specificTagListeners == null) { trackingNewTag = true; - specificTagListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + specificTagListeners = new LinkedList<>(); analysisTagListeners.put(tagKey, specificTagListeners); } - specificTagListeners.add(listener); + Util.addListener(specificTagListeners, listener); if (trackingNewTag) primeCache(); // Someone is interested in something new, so go get it. } } @@ -490,9 +492,9 @@ public synchronized void addAnalysisTagListener(final AnalysisTagListener listen public synchronized void removeAnalysisTagListener(final AnalysisTagListener listener, final String fileExtension, final String typeTag) { if (listener != null) { final String tagKey = typeTag + fileExtension; - Set specificTagListeners = analysisTagListeners.get(tagKey); + List> specificTagListeners = analysisTagListeners.get(tagKey); if (specificTagListeners != null) { - specificTagListeners.remove(listener); + Util.removeListener(specificTagListeners, listener); if (specificTagListeners.isEmpty()) { // No listeners left of this type, remove the parent entry. analysisTagListeners.remove(tagKey); } @@ -508,11 +510,11 @@ public synchronized void removeAnalysisTagListener(final AnalysisTagListener lis * @return the listeners that are currently registered for track analysis updates, indexed by typeTag + fileExtension */ @API(status = API.Status.STABLE) - public Map> getTagListeners() { + public synchronized Map> getTagListeners() { // Make a copy so callers get an immutable snapshot of the current state. final Map> result = new HashMap<>(); - for (Map.Entry> entry : new HashMap<>(analysisTagListeners).entrySet()) { - result.put(entry.getKey(), Set.copyOf(entry.getValue())); + for (Map.Entry>> entry : new HashMap<>(analysisTagListeners).entrySet()) { + result.put(entry.getKey(), Collections.unmodifiableSet(Util.gatherListeners(entry.getValue()))); } return Collections.unmodifiableMap(result); @@ -527,10 +529,10 @@ public Map> getTagListeners() { * @param taggedSection the new parsed track analysis information, if any */ private void deliverAnalysisTagUpdate(final int player, final String fileExtension, final String typeTag, final RekordboxAnlz.TaggedSection taggedSection) { - final Set currentListeners = analysisTagListeners.get(typeTag + fileExtension); + final List> currentListeners = analysisTagListeners.get(typeTag + fileExtension); if (currentListeners != null) { - // Iterate over a copy to avoid concurrent modification issues. - final Set listeners = new HashSet<>(currentListeners); + // Iterate over a copy to avoid concurrent modification issues, and filter out garbage-collected ones. + final Set listeners = Util.gatherListeners(currentListeners); if (!listeners.isEmpty()) { SwingUtilities.invokeLater(() -> { final AnalysisTagUpdate update = new AnalysisTagUpdate(player, fileExtension, typeTag, taggedSection); diff --git a/src/main/java/org/deepsymmetry/beatlink/data/ArtFinder.java b/src/main/java/org/deepsymmetry/beatlink/data/ArtFinder.java index adfed9f..6c74dc7 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/ArtFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/ArtFinder.java @@ -8,6 +8,7 @@ import javax.swing.*; import java.io.IOException; +import java.lang.ref.WeakReference; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.LinkedBlockingDeque; @@ -496,13 +497,13 @@ private AlbumArt findArtInMemoryCaches(DataReference artReference) { /** * Keeps track of the registered track metadata update listeners. */ - private final Set artListeners = - Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> artListeners = new LinkedList<>(); /** *

Adds the specified album art listener to receive updates when the album art for a player changes. * If {@code listener} is {@code null} or already present in the set of registered listeners, no exception is - * thrown and no action is performed.

+ * thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, updates are delivered to listeners directly on the thread that is receiving packets * from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -516,10 +517,8 @@ private AlbumArt findArtInMemoryCaches(DataReference artReference) { * @param listener the album art update listener to add */ @API(status = API.Status.STABLE) - public void addAlbumArtListener(AlbumArtListener listener) { - if (listener != null) { - artListeners.add(listener); - } + public synchronized void addAlbumArtListener(AlbumArtListener listener) { + Util.addListener(artListeners, listener); } /** @@ -530,10 +529,8 @@ public void addAlbumArtListener(AlbumArtListener listener) { * @param listener the album art update listener to remove */ @API(status = API.Status.STABLE) - public void removeAlbumArtListener(AlbumArtListener listener) { - if (listener != null) { - artListeners.remove(listener); - } + public synchronized void removeAlbumArtListener(AlbumArtListener listener) { + Util.removeListener(artListeners, listener); } /** @@ -542,18 +539,19 @@ public void removeAlbumArtListener(AlbumArtListener listener) { * @return the listeners that are currently registered for album art updates */ @API(status = API.Status.STABLE) - public Set getAlbumArtListeners() { + public synchronized Set getAlbumArtListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(artListeners); + return Collections.unmodifiableSet(Util.gatherListeners(artListeners)); } /** * Send an album art update announcement to all registered listeners. */ private void deliverAlbumArtUpdate(int player, AlbumArt art) { - if (!getAlbumArtListeners().isEmpty()) { + Set listeners = getAlbumArtListeners(); + if (!listeners.isEmpty()) { final AlbumArtUpdate update = new AlbumArtUpdate(player, art); - for (final AlbumArtListener listener : getAlbumArtListeners()) { + for (final AlbumArtListener listener : listeners) { try { listener.albumArtChanged(update); diff --git a/src/main/java/org/deepsymmetry/beatlink/data/BeatGridFinder.java b/src/main/java/org/deepsymmetry/beatlink/data/BeatGridFinder.java index 99d8591..4bb0c25 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/BeatGridFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/BeatGridFinder.java @@ -11,6 +11,7 @@ import javax.swing.*; import java.io.IOException; +import java.lang.ref.WeakReference; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.LinkedBlockingDeque; @@ -304,12 +305,13 @@ BeatGrid getBeatGrid(int rekordboxId, SlotReference slot, Client client) /** * Keeps track of the registered beat grid listeners. */ - private final Set beatGridListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> beatGridListeners = new LinkedList<>(); /** *

Adds the specified beat grid listener to receive updates when the beat grid information for a player changes. * If {@code listener} is {@code null} or already present in the set of registered listeners, no exception is - * thrown and no action is performed.

+ * thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

To reduce latency, updates are delivered to listeners directly on the thread that is receiving packets * from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -323,10 +325,8 @@ BeatGrid getBeatGrid(int rekordboxId, SlotReference slot, Client client) * @param listener the album art update listener to add */ @API(status = API.Status.STABLE) - public void addBeatGridListener(BeatGridListener listener) { - if (listener != null) { - beatGridListeners.add(listener); - } + public synchronized void addBeatGridListener(BeatGridListener listener) { + Util.addListener(beatGridListeners, listener); } /** @@ -337,10 +337,8 @@ public void addBeatGridListener(BeatGridListener listener) { * @param listener the waveform listener to remove */ @API(status = API.Status.STABLE) - public void removeBeatGridListener(BeatGridListener listener) { - if (listener != null) { - beatGridListeners.remove(listener); - } + public synchronized void removeBeatGridListener(BeatGridListener listener) { + Util.removeListener(beatGridListeners, listener); } /** @@ -349,9 +347,9 @@ public void removeBeatGridListener(BeatGridListener listener) { * @return the listeners that are currently registered for beat grid updates */ @API(status = API.Status.STABLE) - public Set getBeatGridListeners() { + public synchronized Set getBeatGridListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(beatGridListeners); + return Collections.unmodifiableSet(Util.gatherListeners(beatGridListeners)); } /** @@ -361,9 +359,10 @@ public Set getBeatGridListeners() { * @param beatGrid the new beat grid associated with that player, if any */ private void deliverBeatGridUpdate(int player, BeatGrid beatGrid) { - if (!getBeatGridListeners().isEmpty()) { + final Set listeners = getBeatGridListeners(); + if (!listeners.isEmpty()) { final BeatGridUpdate update = new BeatGridUpdate(player, beatGrid); - for (final BeatGridListener listener : getBeatGridListeners()) { + for (final BeatGridListener listener : listeners) { try { listener.beatGridChanged(update); diff --git a/src/main/java/org/deepsymmetry/beatlink/data/CrateDigger.java b/src/main/java/org/deepsymmetry/beatlink/data/CrateDigger.java index 8cef59b..1536739 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/CrateDigger.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/CrateDigger.java @@ -13,6 +13,7 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.lang.ref.WeakReference; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; @@ -653,14 +654,14 @@ static File createDownloadDirectory() { /** * Keeps track of the registered database listeners. */ - private final Set dbListeners = - Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> dbListeners = new LinkedList<>(); /** * Adds the specified database listener to receive updates when a rekordbox database has been obtained for a * media slot, or when the underlying media for a database has been unmounted, so it is no longer relevant. * If {@code listener} is {@code null} or already present in the set of registered listeners, no exception is - * thrown and no action is performed. + * thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references. * *

To reduce latency, updates are delivered to listeners directly on the thread that is receiving packets * from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -674,10 +675,8 @@ static File createDownloadDirectory() { * @param listener the database update listener to add */ @API(status = API.Status.STABLE) - public void addDatabaseListener(DatabaseListener listener) { - if (listener != null) { - dbListeners.add(listener); - } + public synchronized void addDatabaseListener(DatabaseListener listener) { + Util.addListener(dbListeners, listener); } /** @@ -688,16 +687,19 @@ public void addDatabaseListener(DatabaseListener listener) { * @param listener the database update listener to remove */ @API(status = API.Status.STABLE) - public void removeDatabaseListener(DatabaseListener listener) { - if (listener != null) { - dbListeners.remove(listener); - } + public synchronized void removeDatabaseListener(DatabaseListener listener) { + Util.removeListener(dbListeners, listener); } + /** + * Get the set of currently-registered database listeners. + * + * @return the listeners that are currently registered for database updates + */ @API(status = API.Status.STABLE) - public Set getDatabaseListeners() { + public synchronized Set getDatabaseListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(dbListeners); + return Collections.unmodifiableSet(Util.gatherListeners(dbListeners)); } /** diff --git a/src/main/java/org/deepsymmetry/beatlink/data/MetadataFinder.java b/src/main/java/org/deepsymmetry/beatlink/data/MetadataFinder.java index 7273e0a..22f2254 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/MetadataFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/MetadataFinder.java @@ -12,6 +12,7 @@ import javax.swing.*; import java.io.IOException; +import java.lang.ref.WeakReference; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.LinkedBlockingDeque; @@ -374,7 +375,7 @@ public boolean isRunning() { /** * Check whether we are configured to use metadata only from caches and downloaded metadata exports, - * never actively requesting it from a player. Note that this will implicitly mean all of the metadata-related + * never actively requesting it from a player. Note that this will implicitly mean all the metadata-related * finders ({@link ArtFinder}, {@link BeatGridFinder}, and {@link WaveformFinder}) are in passive mode as well, * because their activity is triggered by the availability of new track metadata. * @@ -389,7 +390,7 @@ public boolean isPassive() { /** * Set whether we are configured to use metadata only from caches or downloaded metadata exports, * never actively requesting it from a player. - * Note that this will implicitly put all of the metadata-related finders ({@link ArtFinder}, {@link BeatGridFinder}, + * Note that this will implicitly put all the metadata-related finders ({@link ArtFinder}, {@link BeatGridFinder}, * and {@link WaveformFinder}) into a passive mode as well, because their activity is triggered by the availability * of new track metadata. * @@ -603,12 +604,13 @@ public MediaDetails getMediaDetailsFor(SlotReference slot) { /** * Keeps track of the registered mount update listeners. */ - private final Set mountListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> mountListeners = new LinkedList<>(); /** * Adds the specified mount update listener to receive updates when media is mounted or unmounted by any player. * If {@code listener} is {@code null} or already present in the set of registered listeners, no exception is - * thrown and no action is performed. + * thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references. * *

Note that at the time a mount is detected, we will not yet know any details about the mounted media. * If {@code listener} also implements {@link MediaDetailsListener}, then as soon as the media details have @@ -627,10 +629,8 @@ public MediaDetails getMediaDetailsFor(SlotReference slot) { * @param listener the mount update listener to add */ @API(status = API.Status.STABLE) - public void addMountListener(MountListener listener) { - if (listener != null) { - mountListeners.add(listener); - } + public synchronized void addMountListener(MountListener listener) { + Util.addListener(mountListeners, listener); } /** @@ -641,10 +641,8 @@ public void addMountListener(MountListener listener) { * @param listener the mount update listener to remove */ @API(status = API.Status.STABLE) - public void removeMountListener(MountListener listener) { - if (listener != null) { - mountListeners.remove(listener); - } + public synchronized void removeMountListener(MountListener listener) { + Util.removeListener(mountListeners, listener); } /** @@ -653,9 +651,9 @@ public void removeMountListener(MountListener listener) { * @return the listeners that are currently registered for mount updates */ @API(status = API.Status.STABLE) - public Set getMountListeners() { + public synchronized Set getMountListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(mountListeners); + return Collections.unmodifiableSet(Util.gatherListeners(mountListeners)); } /** @@ -689,12 +687,13 @@ private void deliverMountUpdate(SlotReference slot, boolean mounted) { /** * Keeps track of the registered track metadata update listeners. */ - private final Set trackListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> trackListeners = new LinkedList<>(); /** * Adds the specified track metadata listener to receive updates when the track metadata for a player changes. * If {@code listener} is {@code null} or already present in the set of registered listeners, no exception is - * thrown and no action is performed. + * thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references. * *

To reduce latency, updates are delivered to listeners directly on the thread that is receiving packets * from the network, so if you want to interact with user interface objects in listener methods, you need to use @@ -708,10 +707,8 @@ private void deliverMountUpdate(SlotReference slot, boolean mounted) { * @param listener the track metadata update listener to add */ @API(status = API.Status.STABLE) - public void addTrackMetadataListener(TrackMetadataListener listener) { - if (listener != null) { - trackListeners.add(listener); - } + public synchronized void addTrackMetadataListener(TrackMetadataListener listener) { + Util.addListener(trackListeners, listener); } /** @@ -722,10 +719,8 @@ public void addTrackMetadataListener(TrackMetadataListener listener) { * @param listener the track metadata update listener to remove */ @API(status = API.Status.STABLE) - public void removeTrackMetadataListener(TrackMetadataListener listener) { - if (listener != null) { - trackListeners.remove(listener); - } + public synchronized void removeTrackMetadataListener(TrackMetadataListener listener) { + Util.removeListener(trackListeners, listener); } /** @@ -734,9 +729,9 @@ public void removeTrackMetadataListener(TrackMetadataListener listener) { * @return the listeners that are currently registered for track metadata updates */ @API(status = API.Status.STABLE) - public Set getTrackMetadataListeners() { + public synchronized Set getTrackMetadataListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(trackListeners); + return Collections.unmodifiableSet(Util.gatherListeners(trackListeners)); } /** diff --git a/src/main/java/org/deepsymmetry/beatlink/data/SignatureFinder.java b/src/main/java/org/deepsymmetry/beatlink/data/SignatureFinder.java index afa7a68..26e97ff 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/SignatureFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/SignatureFinder.java @@ -6,6 +6,7 @@ import org.slf4j.LoggerFactory; import javax.swing.*; +import java.lang.ref.WeakReference; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -174,12 +175,13 @@ public String getLatestSignatureFor(DeviceUpdate update) { /** * Keeps track of the registered signature listeners. */ - private final Set signatureListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> signatureListeners = new LinkedList<>(); /** *

Adds the specified signature listener to receive updates when the track signature for a player changes. * If {@code listener} is {@code null} or already present in the set of registered listeners, no exception is - * thrown and no action is performed.

+ * thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

Updates are delivered to listeners on the Swing Event Dispatch thread, so it is safe to interact with * user interface elements within the event handler.

@@ -191,10 +193,8 @@ public String getLatestSignatureFor(DeviceUpdate update) { * @param listener the track signature update listener to add */ @API(status = API.Status.STABLE) - public void addSignatureListener(SignatureListener listener) { - if (listener != null) { - signatureListeners.add(listener); - } + public synchronized void addSignatureListener(SignatureListener listener) { + Util.addListener(signatureListeners, listener); } /** @@ -205,10 +205,8 @@ public void addSignatureListener(SignatureListener listener) { * @param listener the track signature update listener to remove */ @API(status = API.Status.STABLE) - public void removeSignatureListener(SignatureListener listener) { - if (listener != null) { - signatureListeners.remove(listener); - } + public synchronized void removeSignatureListener(SignatureListener listener) { + Util.removeListener(signatureListeners, listener); } /** @@ -217,9 +215,9 @@ public void removeSignatureListener(SignatureListener listener) { * @return the listeners that are currently registered for track signature updates */ @API(status = API.Status.STABLE) - public Set getSignatureListeners() { + public synchronized Set getSignatureListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(signatureListeners); + return Collections.unmodifiableSet(Util.gatherListeners(signatureListeners)); } private void deliverSignatureUpdate(final int player, final String signature) { diff --git a/src/main/java/org/deepsymmetry/beatlink/data/TimeFinder.java b/src/main/java/org/deepsymmetry/beatlink/data/TimeFinder.java index b84adcd..059e1c4 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/TimeFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/TimeFinder.java @@ -5,13 +5,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; +import java.lang.ref.WeakReference; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; /** *

Watches the beat packets and transport information contained in player status update to infer the current @@ -286,29 +286,49 @@ public long getTimeFor(DeviceUpdate update) { } /** - * Keeps track of the listeners that have registered interest in closely following track playback for a particular - * player. The keys are the listener interface, and the values are the last update that was sent to that - * listener. If no information was known during the last update, the special value {@link #NO_INFORMATION} is - * used to represent it, rather than trying to store a {@code null} value in the hash map. + * Used to keep track of the information we need to about listeners registered for track position updates, + * without preventing their garbage collection. */ - private final ConcurrentHashMap trackPositionListeners = new ConcurrentHashMap<>(); + private static class TrackPositionListenerRecord { - /** - * Keeps track of the player numbers that registered track position listeners are interested in. - */ - private final ConcurrentHashMap listenerPlayerNumbers = new ConcurrentHashMap<>(); + /** + * Holds a reference to the registered listener until it gets garbage collected. + */ + final WeakReference listener; + + /** + * Keeps track of the player number that the listener is interested in. + */ + final int playerNumber; + + /** + * Keeps track of the update we last sent to this listener, if any. + */ + final AtomicReference lastUpdateSent = new AtomicReference<>(); + + /** + * Constructor sets up the immutable fields. + * + * @param listener the listener registered to receive updates + * @param player the device number it is interested in updates for + */ + TrackPositionListenerRecord(TrackPositionListener listener, int player) { + this.listener = new WeakReference<>(listener); + playerNumber = player; + } + } /** - * This is used to represent the fact that we have told a listener that there is no information for it, since - * we can't actually store a {@code null} value in a {@link ConcurrentHashMap}. + * Keeps track of the listeners that have registered interest in closely following track playback for a particular + * player. */ - private final TrackPositionUpdate NO_INFORMATION = new TrackPositionUpdate(0, 0, 0, - false, false, 0, false, null, false, false); + List trackPositionListeners = new LinkedList<>(); /** * Add a listener that wants to closely follow track playback for a particular player. The listener will be called * as soon as there is an initial {@link TrackPositionUpdate} for the specified player, and whenever there is an - * unexpected change in playback position, speed, or state on that player. + * unexpected change in playback position, speed, or state on that player. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references. *

* To help the listener orient itself, it is sent a {@link TrackPositionListener#movementChanged(TrackPositionUpdate)} * message immediately upon registration to report the current playback position, even if none is known (in which @@ -321,11 +341,24 @@ public long getTimeFor(DeviceUpdate update) { * @param listener the interface that will be called when there are changes in track playback on the player */ @API(status = API.Status.STABLE) - public void addTrackPositionListener(int player, TrackPositionListener listener) { - listenerPlayerNumbers.put(listener, player); - TrackPositionUpdate currentPosition = positions.get(player); - trackPositionListeners.put(listener, currentPosition == null? NO_INFORMATION : currentPosition); - listener.movementChanged(currentPosition); // If this throws an exception, the caller will catch it. + public synchronized void addTrackPositionListener(int player, TrackPositionListener listener) { + Iterator iterator = trackPositionListeners.iterator(); + while (iterator.hasNext()) { + TrackPositionListenerRecord listenerRecord = iterator.next(); + TrackPositionListener currentListener = listenerRecord.listener.get(); + + if (currentListener == null || currentListener == listener) { + iterator.remove(); // We found a garbage-collected listener, or former registration of same one. + } + } + + if (listener != null) { // We have a new listener to register + TrackPositionListenerRecord record = new TrackPositionListenerRecord(listener, player); + TrackPositionUpdate currentPosition = positions.get(player); + record.lastUpdateSent.set(currentPosition); + trackPositionListeners.add(record); + listener.movementChanged(currentPosition); // If this throws an exception, the caller will catch it. + } } /** @@ -334,9 +367,12 @@ public void addTrackPositionListener(int player, TrackPositionListener listener) * @param listener the interface that will no longer be called for changes in track playback */ @API(status = API.Status.STABLE) - public void removeTrackPositionListener(TrackPositionListener listener) { - trackPositionListeners.remove(listener); - listenerPlayerNumbers.remove(listener); + public synchronized void removeTrackPositionListener(TrackPositionListener listener) { + Iterator iterator = trackPositionListeners.iterator(); + while (iterator.hasNext()) { + TrackPositionListener currentListener = iterator.next().listener.get(); + if (currentListener == listener || currentListener == null) iterator.remove(); + } } /** @@ -405,6 +441,26 @@ private boolean pitchesDiffer(TrackPositionUpdate lastUpdate, TrackPositionUpdat } } + /** + * Gather a copy of the surviving registered listener records. + * + * @return information about the listeners that have not yet been garbage collected. + */ + private synchronized List getTrackPositionListeners() { + List result = new LinkedList<>(); + Iterator iterator = trackPositionListeners.iterator(); + while (iterator.hasNext()) { + TrackPositionListenerRecord record = iterator.next(); + if (record.listener.get() == null) { + iterator.remove(); + } else { + result.add(record); + } + } + return result; + + } + /** * Check if the current position tracking information for a player represents a significant change compared to * what a listener was last informed to expect, and if so, send another update. If this is a definitive update @@ -417,40 +473,40 @@ private boolean pitchesDiffer(TrackPositionUpdate lastUpdate, TrackPositionUpdat */ private void updateListenersIfNeeded(int player, TrackPositionUpdate update, Beat beat) { // Iterate over a copy to avoid issues with concurrent modification - for (Map.Entry entry : new HashMap<>(trackPositionListeners).entrySet()) { - if (player == listenerPlayerNumbers.get(entry.getKey())) { // This listener is interested in this player - if (update == null) { // We are reporting a loss of information - if (entry.getValue() != NO_INFORMATION) { - if (trackPositionListeners.replace(entry.getKey(), entry.getValue(), NO_INFORMATION)) { + for (TrackPositionListenerRecord record : getTrackPositionListeners()) { + TrackPositionListener listener = record.listener.get(); + if (listener != null) { // The listener was not garbage collected after the list was gathered. + if (player == record.playerNumber) { // This listener is interested in this player + if (update == null) { // We are reporting a loss of information + if (record.lastUpdateSent.getAndSet(null) != null) { try { - entry.getKey().movementChanged(null); + listener.movementChanged(null); } catch (Throwable t) { logger.warn("Problem delivering null movementChanged update", t); } } - } - } else { // We have some information, see if it is a significant change from what was last reported - final TrackPositionUpdate lastUpdate = entry.getValue(); - if (lastUpdate == NO_INFORMATION || - lastUpdate.playing != update.playing || - pitchesDiffer(lastUpdate, update) || - interpolationsDisagree(lastUpdate, update)) { - if (trackPositionListeners.replace(entry.getKey(), entry.getValue(), update)) { + } else { // We have some information, see if it is a significant change from what was last reported + final TrackPositionUpdate lastUpdate = record.lastUpdateSent.get(); + if (lastUpdate == null || + lastUpdate.playing != update.playing || + pitchesDiffer(lastUpdate, update) || + interpolationsDisagree(lastUpdate, update)) { try { - entry.getKey().movementChanged(update); + listener.movementChanged(update); } catch (Throwable t) { logger.warn("Problem delivering movementChanged update", t); } + record.lastUpdateSent.set(update); } - } - // And regardless of whether this was a significant change, if this was a new beat and the listener - // implements the interface that requests all beats, send that information. - if (update.fromBeat && entry.getKey() instanceof TrackPositionBeatListener) { - try { - ((TrackPositionBeatListener) entry.getKey()).newBeat(beat, update); - } catch (Throwable t) { - logger.warn("Problem delivering newBeat update", t); + // And regardless of whether this was a significant change, if this was a new beat and the listener + // implements the interface that requests all beats, send that information. + if (update.fromBeat && listener instanceof TrackPositionBeatListener) { + try { + ((TrackPositionBeatListener) listener).newBeat(beat, update); + } catch (Throwable t) { + logger.warn("Problem delivering newBeat update", t); + } } } } diff --git a/src/main/java/org/deepsymmetry/beatlink/data/WaveformFinder.java b/src/main/java/org/deepsymmetry/beatlink/data/WaveformFinder.java index 728f919..d6d97bc 100644 --- a/src/main/java/org/deepsymmetry/beatlink/data/WaveformFinder.java +++ b/src/main/java/org/deepsymmetry/beatlink/data/WaveformFinder.java @@ -8,6 +8,7 @@ import javax.swing.*; import java.io.IOException; +import java.lang.ref.WeakReference; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.LinkedBlockingDeque; @@ -605,12 +606,13 @@ WaveformDetail getWaveformDetail(int rekordboxId, SlotReference slot, Client cli /** * Keeps track of the registered waveform listeners. */ - private final Set waveformListeners = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final List> waveformListeners = new LinkedList<>(); /** *

Adds the specified waveform listener to receive updates when the waveform information for a player changes. * If {@code listener} is {@code null} or already present in the set of registered listeners, no exception is - * thrown and no action is performed.

+ * thrown and no action is performed. Presence on a listener list does not + * prevent an object from being garbage-collected if it has no other references.

* *

Updates are delivered to listeners on the Swing Event Dispatch thread, so it is safe to interact with * user interface elements within the event handler.

@@ -622,10 +624,8 @@ WaveformDetail getWaveformDetail(int rekordboxId, SlotReference slot, Client cli * @param listener the waveform update listener to add */ @API(status = API.Status.STABLE) - public void addWaveformListener(WaveformListener listener) { - if (listener != null) { - waveformListeners.add(listener); - } + public synchronized void addWaveformListener(WaveformListener listener) { + Util.addListener(waveformListeners, listener); } /** @@ -636,10 +636,8 @@ public void addWaveformListener(WaveformListener listener) { * @param listener the waveform update listener to remove */ @API(status = API.Status.STABLE) - public void removeWaveformListener(WaveformListener listener) { - if (listener != null) { - waveformListeners.remove(listener); - } + public synchronized void removeWaveformListener(WaveformListener listener) { + Util.removeListener(waveformListeners, listener); } /** @@ -648,9 +646,9 @@ public void removeWaveformListener(WaveformListener listener) { * @return the listeners that are currently registered for waveform updates */ @API(status = API.Status.STABLE) - public Set getWaveformListeners() { + public synchronized Set getWaveformListeners() { // Make a copy so callers get an immutable snapshot of the current state. - return Set.copyOf(waveformListeners); + return Collections.unmodifiableSet(Util.gatherListeners(waveformListeners)); } /**