Use weak references for listener lists, closes #85

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

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
 

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<DeviceAnnouncementListener> deviceListeners =
-            Collections.newSetFromMap(new ConcurrentHashMap<>());
+    private final List<WeakReference<DeviceAnnouncementListener>> 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.
      *
      * <p>Device announcements are delivered to listeners on the
      * <a href="https://docs.oracle.com/javase/tutorial/uiswing/concurrency/dispatch.html">Event Dispatch thread</a>,
@@ -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<DeviceAnnouncementListener> getDeviceAnnouncementListeners() {
+    public synchronized Set<DeviceAnnouncementListener> getDeviceAnnouncementListeners() {
         // Make a copy so callers get an immutable snapshot of the current state.
-        return Set.copyOf(deviceListeners);
+        return Collections.unmodifiableSet(Util.gatherListeners(deviceListeners));
     }
 
     /**

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<LifecycleListener> lifecycleListeners = Collections.newSetFromMap(new ConcurrentHashMap<>());
+    private final List<WeakReference<LifecycleListener>> lifecycleListeners = new LinkedList<>();
 
     /**
      * <p>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.</p>
+     * 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.</p>
      *
      * <p>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<LifecycleListener> getLifecycleListeners() {
+    public synchronized Set<LifecycleListener> 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));
     }
 
     /**

src/main/java/org/deepsymmetry/beatlink/Util.java

@@ -8,13 +8,15 @@
 
 import java.awt.*;
 import java.io.IOException;
+import java.lang.ref.WeakReference;
 import java.net.DatagramPacket;
 import java.net.InetAddress;
 import java.net.InterfaceAddress;
 import java.net.NetworkInterface;
 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 <T> the type of listeners weakly referenced by the list
+     */
+    public static <T> void addListener(List<WeakReference<T>> listenerList, T listener) {
+        Iterator<WeakReference<T>> 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 <T> the type of listeners weakly referenced by the list
+     */
+    public static <T> void removeListener(List<WeakReference<T>> listenerList, T listener) {
+        Iterator<WeakReference<T>> 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 <T> the type of listeners weakly referenced by the list
+     *
+     * @return the listeners that were registered and have not been garbage collected
+     */
+    public static <T> Set<T> gatherListeners(List<WeakReference<T>> listenerList) {
+        Set<T> result = new HashSet<>();
+        Iterator<WeakReference<T>> 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.
      */

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());

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<String, Set<AnalysisTagListener>> analysisTagListeners = new ConcurrentHashMap<>();
+    private final Map<String, List<WeakReference<AnalysisTagListener>>> analysisTagListeners = new ConcurrentHashMap<>();
 
     /**
      * <p>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.</p>
+     * 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.</p>
      *
      * <p>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.</p>
@@ -466,13 +468,13 @@ public synchronized void addAnalysisTagListener(final AnalysisTagListener listen
         if (listener != null) {
             final String tagKey = typeTag + fileExtension;
             boolean trackingNewTag = false;
-            Set<AnalysisTagListener> specificTagListeners = analysisTagListeners.get(tagKey);
+            List<WeakReference<AnalysisTagListener>> 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<AnalysisTagListener> specificTagListeners = analysisTagListeners.get(tagKey);
+            List<WeakReference<AnalysisTagListener>> 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<String, Set<AnalysisTagListener>> getTagListeners() {
+    public synchronized Map<String, Set<AnalysisTagListener>> getTagListeners() {
         // Make a copy so callers get an immutable snapshot of the current state.
         final Map<String, Set<AnalysisTagListener>> result = new HashMap<>();
-        for (Map.Entry<String, Set<AnalysisTagListener>> entry : new HashMap<>(analysisTagListeners).entrySet()) {
-            result.put(entry.getKey(), Set.copyOf(entry.getValue()));
+        for (Map.Entry<String, List<WeakReference<AnalysisTagListener>>> 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<String, Set<AnalysisTagListener>> 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<AnalysisTagListener> currentListeners = analysisTagListeners.get(typeTag + fileExtension);
+        final List<WeakReference<AnalysisTagListener>> currentListeners = analysisTagListeners.get(typeTag + fileExtension);
         if (currentListeners != null) {
-            // Iterate over a copy to avoid concurrent modification issues.
-            final Set<AnalysisTagListener> listeners = new HashSet<>(currentListeners);
+            // Iterate over a copy to avoid concurrent modification issues, and filter out garbage-collected ones.
+            final Set<AnalysisTagListener> listeners = Util.gatherListeners(currentListeners);
             if (!listeners.isEmpty()) {
                 SwingUtilities.invokeLater(() -> {
                     final AnalysisTagUpdate update = new AnalysisTagUpdate(player, fileExtension, typeTag, taggedSection);