Document and enforce "one request per Listener" rule

The API and implementation of NsdManager imply that a separate
Listener is to be used for each active registration or discovery
request.  This isn't formally documented or properly enforced, and
weird and unpredictable things happen if an application uses a
Listener for more than one request at a time.

Update documentation to make this an explicit requirement.

Enforce the restriction when a new request is submitted for
processing; if the Listener is already being used to track an active
request, throw an exception.

Document the fact that apps should unregister services and cancel
service discoveries when the app is stopped (in KitKat and prior
releases, they'll leak if this isn't done.)

Re-order "release the Listener" operation to occur before the Listener
callback, so that the Listener can be reused by the application once
the callback has been entered - this eliminates a race condition.
Document this.

Pass 2: typos, added documentation about API level, changed to using
an explicitly defined return value for "busy listener".

Bug: 13512512
Change-Id: Ic164110759204b27d8a14376777b593ebe1865fa

1 files changed, 58 insertions, 24 deletions

diff --git a/core/java/android/net/nsd/NsdManager.java b/core/java/android/net/nsd/NsdManager.java
index d8e8e2c..377ed88 100644
--- a/core/java/android/net/nsd/NsdManager.java
+++ b/core/java/android/net/nsd/NsdManager.java
@@ -211,6 +211,7 @@ public final class NsdManager {
     private Context mContext;
 
     private static final int INVALID_LISTENER_KEY = 0;
+    private static final int BUSY_LISTENER_KEY = -1;
     private int mListenerKey = 1;
     private final SparseArray mListenerMap = new SparseArray();
     private final SparseArray<NsdServiceInfo> mServiceMap = new SparseArray<NsdServiceInfo>();
@@ -317,71 +318,74 @@ public final class NsdManager {
                 Log.d(TAG, "Stale key " + message.arg2);
                 return;
             }
-            boolean listenerRemove = true;
             NsdServiceInfo ns = getNsdService(message.arg2);
             switch (message.what) {
                 case DISCOVER_SERVICES_STARTED:
                     String s = getNsdServiceInfoType((NsdServiceInfo) message.obj);
                     ((DiscoveryListener) listener).onDiscoveryStarted(s);
-                    // Keep listener until stop discovery
-                    listenerRemove = false;
                     break;
                 case DISCOVER_SERVICES_FAILED:
+                    removeListener(message.arg2);
                     ((DiscoveryListener) listener).onStartDiscoveryFailed(getNsdServiceInfoType(ns),
                             message.arg1);
                     break;
                 case SERVICE_FOUND:
                     ((DiscoveryListener) listener).onServiceFound((NsdServiceInfo) message.obj);
-                    // Keep listener until stop discovery
-                    listenerRemove = false;
                     break;
                 case SERVICE_LOST:
                     ((DiscoveryListener) listener).onServiceLost((NsdServiceInfo) message.obj);
-                    // Keep listener until stop discovery
-                    listenerRemove = false;
                     break;
                 case STOP_DISCOVERY_FAILED:
+                    removeListener(message.arg2);
                     ((DiscoveryListener) listener).onStopDiscoveryFailed(getNsdServiceInfoType(ns),
                             message.arg1);
                     break;
                 case STOP_DISCOVERY_SUCCEEDED:
+                    removeListener(message.arg2);
                     ((DiscoveryListener) listener).onDiscoveryStopped(getNsdServiceInfoType(ns));
                     break;
                 case REGISTER_SERVICE_FAILED:
+                    removeListener(message.arg2);
                     ((RegistrationListener) listener).onRegistrationFailed(ns, message.arg1);
                     break;
                 case REGISTER_SERVICE_SUCCEEDED:
                     ((RegistrationListener) listener).onServiceRegistered(
                             (NsdServiceInfo) message.obj);
-                    // Keep listener until unregister
-                    listenerRemove = false;
                     break;
                 case UNREGISTER_SERVICE_FAILED:
+                    removeListener(message.arg2);
                     ((RegistrationListener) listener).onUnregistrationFailed(ns, message.arg1);
                     break;
                 case UNREGISTER_SERVICE_SUCCEEDED:
+                    removeListener(message.arg2);
                     ((RegistrationListener) listener).onServiceUnregistered(ns);
                     break;
                 case RESOLVE_SERVICE_FAILED:
+                    removeListener(message.arg2);
                     ((ResolveListener) listener).onResolveFailed(ns, message.arg1);
                     break;
                 case RESOLVE_SERVICE_SUCCEEDED:
+                    removeListener(message.arg2);
                     ((ResolveListener) listener).onServiceResolved((NsdServiceInfo) message.obj);
                     break;
                 default:
                     Log.d(TAG, "Ignored " + message);
                     break;
             }
-            if (listenerRemove) {
-                removeListener(message.arg2);
-            }
         }
     }
 
+    // if the listener is already in the map, reject it.  Otherwise, add it and
+    // return its key.
+
     private int putListener(Object listener, NsdServiceInfo s) {
         if (listener == null) return INVALID_LISTENER_KEY;
         int key;
         synchronized (mMapLock) {
+            int valueIndex = mListenerMap.indexOfValue(listener);
+            if (valueIndex != -1) {
+                return BUSY_LISTENER_KEY;
+            }
             do {
                 key = mListenerKey++;
             } while (key == INVALID_LISTENER_KEY);
@@ -422,7 +426,6 @@ public final class NsdManager {
         return INVALID_LISTENER_KEY;
     }
 
-
     private String getNsdServiceInfoType(NsdServiceInfo s) {
         if (s == null) return "?";
         return s.getServiceType();
@@ -449,14 +452,18 @@ public final class NsdManager {
      * Register a service to be discovered by other services.
      *
      * <p> The function call immediately returns after sending a request to register service
-     * to the framework. The application is notified of a success to initiate
-     * discovery through the callback {@link RegistrationListener#onServiceRegistered} or a failure
+     * to the framework. The application is notified of a successful registration
+     * through the callback {@link RegistrationListener#onServiceRegistered} or a failure
      * through {@link RegistrationListener#onRegistrationFailed}.
      *
+     * <p> The application should call {@link #unregisterService} when the service
+     * registration is no longer required, and/or whenever the application is stopped.
+     *
      * @param serviceInfo The service being registered
      * @param protocolType The service discovery protocol
      * @param listener The listener notifies of a successful registration and is used to
      * unregister this service through a call on {@link #unregisterService}. Cannot be null.
+     * Cannot be in use for an active service registration.
      */
     public void registerService(NsdServiceInfo serviceInfo, int protocolType,
             RegistrationListener listener) {
@@ -473,8 +480,11 @@ public final class NsdManager {
         if (protocolType != PROTOCOL_DNS_SD) {
             throw new IllegalArgumentException("Unsupported protocol");
         }
-        mAsyncChannel.sendMessage(REGISTER_SERVICE, 0, putListener(listener, serviceInfo),
-                serviceInfo);
+        int key = putListener(listener, serviceInfo);
+        if (key == BUSY_LISTENER_KEY) {
+            throw new IllegalArgumentException("listener already in use");
+        }
+        mAsyncChannel.sendMessage(REGISTER_SERVICE, 0, key, serviceInfo);
     }
 
     /**
@@ -484,7 +494,11 @@ public final class NsdManager {
      *
      * @param listener This should be the listener object that was passed to
      * {@link #registerService}. It identifies the service that should be unregistered
-     * and notifies of a successful unregistration.
+     * and notifies of a successful or unsuccessful unregistration via the listener
+     * callbacks.  In API versions 20 and above, the listener object may be used for
+     * another service registration once the callback has been called.  In API versions <= 19,
+     * there is no entirely reliable way to know when a listener may be re-used, and a new
+     * listener should be created for each service registration request.
      */
     public void unregisterService(RegistrationListener listener) {
         int id = getListenerKey(listener);
@@ -514,12 +528,16 @@ public final class NsdManager {
      * <p> Upon failure to start, service discovery is not active and application does
      * not need to invoke {@link #stopServiceDiscovery}
      *
+     * <p> The application should call {@link #stopServiceDiscovery} when discovery of this
+     * service type is no longer required, and/or whenever the application is paused or
+     * stopped.
+     *
      * @param serviceType The service type being discovered. Examples include "_http._tcp" for
      * http services or "_ipp._tcp" for printers
      * @param protocolType The service discovery protocol
      * @param listener  The listener notifies of a successful discovery and is used
      * to stop discovery on this serviceType through a call on {@link #stopServiceDiscovery}.
-     * Cannot be null.
+     * Cannot be null. Cannot be in use for an active service discovery.
      */
     public void discoverServices(String serviceType, int protocolType, DiscoveryListener listener) {
         if (listener == null) {
@@ -535,11 +553,17 @@ public final class NsdManager {
 
         NsdServiceInfo s = new NsdServiceInfo();
         s.setServiceType(serviceType);
-        mAsyncChannel.sendMessage(DISCOVER_SERVICES, 0, putListener(listener, s), s);
+
+        int key = putListener(listener, s);
+        if (key == BUSY_LISTENER_KEY) {
+            throw new IllegalArgumentException("listener already in use");
+        }
+
+        mAsyncChannel.sendMessage(DISCOVER_SERVICES, 0, key, s);
     }
 
     /**
-     * Stop service discovery initiated with {@link #discoverServices}. An active service
+     * Stop service discovery initiated with {@link #discoverServices}.  An active service
      * discovery is notified to the application with {@link DiscoveryListener#onDiscoveryStarted}
      * and it stays active until the application invokes a stop service discovery. A successful
      * stop is notified to with a call to {@link DiscoveryListener#onDiscoveryStopped}.
@@ -548,7 +572,11 @@ public final class NsdManager {
      * {@link DiscoveryListener#onStopDiscoveryFailed}.
      *
      * @param listener This should be the listener object that was passed to {@link #discoverServices}.
-     * It identifies the discovery that should be stopped and notifies of a successful stop.
+     * It identifies the discovery that should be stopped and notifies of a successful or
+     * unsuccessful stop.  In API versions 20 and above, the listener object may be used for
+     * another service discovery once the callback has been called.  In API versions <= 19,
+     * there is no entirely reliable way to know when a listener may be re-used, and a new
+     * listener should be created for each service discovery request.
      */
     public void stopServiceDiscovery(DiscoveryListener listener) {
         int id = getListenerKey(listener);
@@ -568,6 +596,7 @@ public final class NsdManager {
      *
      * @param serviceInfo service to be resolved
      * @param listener to receive callback upon success or failure. Cannot be null.
+     * Cannot be in use for an active service resolution.
      */
     public void resolveService(NsdServiceInfo serviceInfo, ResolveListener listener) {
         if (TextUtils.isEmpty(serviceInfo.getServiceName()) ||
@@ -577,8 +606,13 @@ public final class NsdManager {
         if (listener == null) {
             throw new IllegalArgumentException("listener cannot be null");
         }
-        mAsyncChannel.sendMessage(RESOLVE_SERVICE, 0, putListener(listener, serviceInfo),
-                serviceInfo);
+
+        int key = putListener(listener, serviceInfo);
+
+        if (key == BUSY_LISTENER_KEY) {
+            throw new IllegalArgumentException("listener already in use");
+        }
+        mAsyncChannel.sendMessage(RESOLVE_SERVICE, 0, key, serviceInfo);
     }
 
     /** Internal use only @hide */