Rewrite NsdManagerDiscoveryAgent lifecycle to avoid listener reuse

app/src/main/java/com/limelight/nvstream/mdns/NsdManagerDiscoveryAgent.java

@@ -21,56 +21,83 @@ import java.util.concurrent.TimeUnit;
 public class NsdManagerDiscoveryAgent extends MdnsDiscoveryAgent {
     private static final String SERVICE_TYPE = "_nvstream._tcp";
     private final NsdManager nsdManager;
-    private boolean discoveryActive;
+    private final Object listenerLock = new Object();
-    private boolean wantsDiscoveryActive;
+    private NsdManager.DiscoveryListener pendingListener;
+    private NsdManager.DiscoveryListener activeListener;
     private final HashMap<String, NsdManager.ServiceInfoCallback> serviceCallbacks = new HashMap<>();
     private final ThreadPoolExecutor executor = new ThreadPoolExecutor(1, 1, 0, TimeUnit.SECONDS, new LinkedBlockingQueue<>());
 
-    private final NsdManager.DiscoveryListener discoveryListener = new NsdManager.DiscoveryListener() {
+    private NsdManager.DiscoveryListener createDiscoveryListener() {
+        return new NsdManager.DiscoveryListener() {
             @Override
             public void onStartDiscoveryFailed(String serviceType, int errorCode) {
-            discoveryActive = false;
                 LimeLog.severe("NSD: Service discovery start failed: " + errorCode);
+
+                // This listener is no longer pending after this failure
+                synchronized (listenerLock) {
+                    if (pendingListener != this) {
+                        return;
+                    }
+
+                    pendingListener = null;
+                }
+
                 listener.notifyDiscoveryFailure(new RuntimeException("onStartDiscoveryFailed(): " + errorCode));
             }
 
             @Override
             public void onStopDiscoveryFailed(String serviceType, int errorCode) {
                 LimeLog.severe("NSD: Service discovery stop failed: " + errorCode);
+
+                // This listener is no longer active after this failure
+                synchronized (listenerLock) {
+                    if (activeListener != this) {
+                        return;
+                    }
+
+                    activeListener = null;
+                }
             }
 
             @Override
             public void onDiscoveryStarted(String serviceType) {
-            discoveryActive = true;
                 LimeLog.info("NSD: Service discovery started");
 
-            // If we were stopped before we could finish starting, stop now
+                synchronized (listenerLock) {
-            if (!wantsDiscoveryActive) {
+                    if (pendingListener != this) {
-                stopDiscovery();
+                        // If we registered another discovery listener in the meantime, stop this one
+                        nsdManager.stopServiceDiscovery(this);
+                        return;
+                    }
+
+                    pendingListener = null;
+                    activeListener = this;
                 }
             }
 
             @Override
             public void onDiscoveryStopped(String serviceType) {
-            discoveryActive = false;
                 LimeLog.info("NSD: Service discovery stopped");
 
-            // If we were started before we could finish stopping, start again now
+                synchronized (listenerLock) {
-            if (wantsDiscoveryActive) {
+                    if (activeListener != this) {
-                startDiscovery(0);
+                        return;
+                    }
+
+                    activeListener = null;
                 }
             }
 
             @Override
             public void onServiceFound(NsdServiceInfo nsdServiceInfo) {
                 // Protect against racing stopDiscovery() call
-            synchronized (serviceCallbacks) {
+                synchronized (listenerLock) {
-                // Bail if we've been stopped
+                    // Ignore callbacks if we're not the active listener
-                if (!wantsDiscoveryActive) {
+                    if (activeListener != this) {
                         return;
                     }
 
-                LimeLog.info("NSD: Machine appeared: "+nsdServiceInfo.getServiceName());
+                    LimeLog.info("NSD: Machine appeared: " + nsdServiceInfo.getServiceName());
 
                     NsdManager.ServiceInfoCallback serviceInfoCallback = new NsdManager.ServiceInfoCallback() {
                         @Override
@@ -81,17 +108,19 @@ public class NsdManagerDiscoveryAgent extends MdnsDiscoveryAgent {
 
                         @Override
                         public void onServiceUpdated(NsdServiceInfo nsdServiceInfo) {
-                        LimeLog.info("NSD: Machine resolved: "+nsdServiceInfo.getServiceName());
+                            LimeLog.info("NSD: Machine resolved: " + nsdServiceInfo.getServiceName());
                             reportNewComputer(nsdServiceInfo.getServiceName(), nsdServiceInfo.getPort(),
                                     getV4Addrs(nsdServiceInfo.getHostAddresses()),
                                     getV6Addrs(nsdServiceInfo.getHostAddresses()));
                         }
 
                         @Override
-                    public void onServiceLost() {}
+                        public void onServiceLost() {
+                        }
 
                         @Override
-                    public void onServiceInfoCallbackUnregistered() {}
+                        public void onServiceInfoCallbackUnregistered() {
+                        }
                     };
 
                     nsdManager.registerServiceInfoCallback(nsdServiceInfo, executor, serviceInfoCallback);
@@ -102,9 +131,9 @@ public class NsdManagerDiscoveryAgent extends MdnsDiscoveryAgent {
             @Override
             public void onServiceLost(NsdServiceInfo nsdServiceInfo) {
                 // Protect against racing stopDiscovery() call
-            synchronized (serviceCallbacks) {
+                synchronized (listenerLock) {
-                // Bail if we've been stopped
+                    // Ignore callbacks if we're not the active listener
-                if (!wantsDiscoveryActive) {
+                    if (activeListener != this) {
                         return;
                     }
 
@@ -117,6 +146,7 @@ public class NsdManagerDiscoveryAgent extends MdnsDiscoveryAgent {
                 }
             }
         };
+    }
 
     public NsdManagerDiscoveryAgent(Context context, MdnsDiscoveryListener listener) {
         super(listener);
@@ -125,23 +155,33 @@ public class NsdManagerDiscoveryAgent extends MdnsDiscoveryAgent {
 
     @Override
     public void startDiscovery(int discoveryIntervalMs) {
-        wantsDiscoveryActive = true;
+        synchronized (listenerLock) {
-
+            // Register a new service discovery listener if there's not already one starting or running
-        // Register the service discovery listener
+            if (pendingListener == null && activeListener == null) {
-        if (!discoveryActive) {
+                pendingListener = createDiscoveryListener();
-            nsdManager.discoverServices(SERVICE_TYPE, NsdManager.PROTOCOL_DNS_SD, discoveryListener);
+                nsdManager.discoverServices(SERVICE_TYPE, NsdManager.PROTOCOL_DNS_SD, pendingListener);
+            }
         }
     }
 
     @Override
     public void stopDiscovery() {
         // Protect against racing ServiceInfoCallback and DiscoveryListener callbacks
-        synchronized (serviceCallbacks) {
+        synchronized (listenerLock) {
-            wantsDiscoveryActive = false;
+            // Clear any pending listener to ensure the discoverStarted() callback
+            // will realize it's gone and stop itself.
+            pendingListener = null;
 
             // Unregister the service discovery listener
-            if (discoveryActive) {
+            if (activeListener != null) {
-                nsdManager.stopServiceDiscovery(discoveryListener);
+                nsdManager.stopServiceDiscovery(activeListener);
+
+                // Even though listener stoppage is asynchronous, the listener is gone as far as
+                // we're concerned. We null this right now to ensure pending callbacks know it's
+                // stopped and startDiscovery() can immediately create a new listener. If we left
+                // it until onDiscoveryStopped() was called, startDiscovery() would get confused
+                // and assume a listener was already running, even though it's stopping.
+                activeListener = null;
             }
 
             // Unregister all service info callbacks