Implements client close and death notification

Bug: 67734082
Test: make from root
Change-Id: Ia139fd6d4bb04c569a9ee3672e21e2700daa40a9

4 files changed, 133 insertions, 19 deletions

diff --git a/core/java/android/hardware/location/ContextHubClient.java b/core/java/android/hardware/location/ContextHubClient.java
index 32ec1138e8f3..52527ed67ae4 100644
--- a/core/java/android/hardware/location/ContextHubClient.java
+++ b/core/java/android/hardware/location/ContextHubClient.java
@@ -18,12 +18,16 @@ package android.hardware.location;
 import android.annotation.RequiresPermission;
 import android.os.RemoteException;
 
+import dalvik.system.CloseGuard;
+
 import java.io.Closeable;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * A class describing a client of the Context Hub Service.
  *
- * Clients can send messages to nanoapps at a Context Hub through this object.
+ * Clients can send messages to nanoapps at a Context Hub through this object. The APIs supported
+ * by this object are thread-safe and can be used without external synchronization.
  *
  * @hide
  */
@@ -43,12 +47,17 @@ public class ContextHubClient implements Closeable {
      */
     private final ContextHubInfo mAttachedHub;
 
+    private final CloseGuard mCloseGuard = CloseGuard.get();
+
+    private final AtomicBoolean mIsClosed = new AtomicBoolean(false);
+
     /* package */ ContextHubClient(
             IContextHubClient clientProxy, IContextHubClientCallback callback,
             ContextHubInfo hubInfo) {
         mClientProxy = clientProxy;
         mCallbackInterface = callback;
         mAttachedHub = hubInfo;
+        mCloseGuard.open("close");
     }
 
     /**
@@ -67,7 +76,14 @@ public class ContextHubClient implements Closeable {
      * All futures messages targeted for this client are dropped at the service.
      */
     public void close() {
-        throw new UnsupportedOperationException("TODO: Implement this");
+        if (!mIsClosed.getAndSet(true)) {
+            mCloseGuard.close();
+            try {
+                mClientProxy.close();
+            } catch (RemoteException e) {
+                throw e.rethrowFromSystemServer();
+            }
+        }
     }
 
     /**
@@ -92,4 +108,16 @@ public class ContextHubClient implements Closeable {
             throw e.rethrowFromSystemServer();
         }
     }
+
+    @Override
+    protected void finalize() throws Throwable {
+        try {
+            if (mCloseGuard != null) {
+                mCloseGuard.warnIfOpen();
+            }
+            close();
+        } finally {
+            super.finalize();
+        }
+    }
 }
diff --git a/core/java/android/hardware/location/IContextHubClient.aidl b/core/java/android/hardware/location/IContextHubClient.aidl
index cdaa15f0d59a..d81126a0ac54 100644
--- a/core/java/android/hardware/location/IContextHubClient.aidl
+++ b/core/java/android/hardware/location/IContextHubClient.aidl
@@ -25,4 +25,7 @@ interface IContextHubClient {
 
     // Sends a message to a nanoapp
     int sendMessageToNanoApp(in NanoAppMessage message);
+
+    // Closes the connection with the Context Hub
+    void close();
 }
diff --git a/services/core/java/com/android/server/location/ContextHubClientBroker.java b/services/core/java/com/android/server/location/ContextHubClientBroker.java
index e78460bf9cfd..276866d2a604 100644
--- a/services/core/java/com/android/server/location/ContextHubClientBroker.java
+++ b/services/core/java/com/android/server/location/ContextHubClientBroker.java
@@ -24,15 +24,21 @@ import android.hardware.location.ContextHubTransaction;
 import android.hardware.location.IContextHubClient;
 import android.hardware.location.IContextHubClientCallback;
 import android.hardware.location.NanoAppMessage;
+import android.os.IBinder;
 import android.os.RemoteException;
 import android.util.Log;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 /**
- * A broker for the ContextHubClient that handles messaging and life-cycle notification callbacks.
+ * A class that acts as a broker for the ContextHubClient, which handles messaging and life-cycle
+ * notification callbacks. This class implements the IContextHubClient object, and the implemented
+ * APIs must be thread-safe.
  *
  * @hide
  */
-public class ContextHubClientBroker extends IContextHubClient.Stub {
+public class ContextHubClientBroker extends IContextHubClient.Stub
+        implements IBinder.DeathRecipient {
     private static final String TAG = "ContextHubClientBroker";
 
     /*
@@ -46,6 +52,11 @@ public class ContextHubClientBroker extends IContextHubClient.Stub {
     private final IContexthub mContextHubProxy;
 
     /*
+     * The manager that registered this client.
+     */
+    private final ContextHubClientManager mClientManager;
+
+    /*
      * The ID of the hub that this client is attached to.
      */
     private final int mAttachedContextHubId;
@@ -60,17 +71,32 @@ public class ContextHubClientBroker extends IContextHubClient.Stub {
      */
     private final IContextHubClientCallback mCallbackInterface;
 
+    /*
+     * false if the connection has been closed by the client, true otherwise.
+     */
+    private final AtomicBoolean mConnectionOpen = new AtomicBoolean(true);
+
     /* package */ ContextHubClientBroker(
-            Context context, IContexthub contextHubProxy, int contextHubId, short hostEndPointId,
-            IContextHubClientCallback callback) {
+            Context context, IContexthub contextHubProxy, ContextHubClientManager clientManager,
+            int contextHubId, short hostEndPointId, IContextHubClientCallback callback) {
         mContext = context;
         mContextHubProxy = contextHubProxy;
+        mClientManager = clientManager;
         mAttachedContextHubId = contextHubId;
         mHostEndPointId = hostEndPointId;
         mCallbackInterface = callback;
     }
 
     /**
+     * Attaches a death recipient for this client
+     *
+     * @throws RemoteException if the client has already died
+     */
+    /* package */ void attachDeathRecipient() throws RemoteException {
+        mCallbackInterface.asBinder().linkToDeath(this, 0 /* flags */);
+    }
+
+    /**
      * Sends from this client to a nanoapp.
      *
      * @param message the message to send
@@ -80,21 +106,49 @@ public class ContextHubClientBroker extends IContextHubClient.Stub {
     @Override
     public int sendMessageToNanoApp(NanoAppMessage message) {
         ContextHubServiceUtil.checkPermissions(mContext);
-        ContextHubMsg messageToNanoApp =
-                ContextHubServiceUtil.createHidlContextHubMessage(mHostEndPointId, message);
 
         int result;
-        try {
-            result = mContextHubProxy.sendMessageToHub(mAttachedContextHubId, messageToNanoApp);
-        } catch (RemoteException e) {
-            Log.e(TAG, "RemoteException in sendMessageToNanoApp (target hub ID = "
-                    + mAttachedContextHubId + ")", e);
+        if (mConnectionOpen.get()) {
+            ContextHubMsg messageToNanoApp = ContextHubServiceUtil.createHidlContextHubMessage(
+                    mHostEndPointId, message);
+
+            try {
+                result = mContextHubProxy.sendMessageToHub(mAttachedContextHubId, messageToNanoApp);
+            } catch (RemoteException e) {
+                Log.e(TAG, "RemoteException in sendMessageToNanoApp (target hub ID = "
+                        + mAttachedContextHubId + ")", e);
+                result = Result.UNKNOWN_FAILURE;
+            }
+        } else {
+            Log.e(TAG, "Failed to send message to nanoapp: client connection is closed");
             result = Result.UNKNOWN_FAILURE;
         }
+
         return ContextHubServiceUtil.toTransactionResult(result);
     }
 
     /**
+     * Closes the connection for this client with the service.
+     */
+    @Override
+    public void close() {
+        if (mConnectionOpen.getAndSet(false)) {
+            mClientManager.unregisterClient(mHostEndPointId);
+        }
+    }
+
+    /**
+     * Invoked when the underlying binder of this broker has died at the client process.
+     */
+    public void binderDied() {
+        try {
+            IContextHubClient.Stub.asInterface(this).close();
+        } catch (RemoteException e) {
+            Log.e(TAG, "RemoteException while closing client on death", e);
+        }
+    }
+
+    /**
      * @return the ID of the context hub this client is attached to
      */
     /* package */ int getAttachedContextHubId() {
@@ -114,11 +168,13 @@ public class ContextHubClientBroker extends IContextHubClient.Stub {
      * @param message the message that came from a nanoapp
      */
     /* package */ void sendMessageToClient(NanoAppMessage message) {
-        try {
-            mCallbackInterface.onMessageFromNanoApp(message);
-        } catch (RemoteException e) {
-            Log.e(TAG, "RemoteException while sending message to client (host endpoint ID = "
-                    + mHostEndPointId + ")", e);
+        if (mConnectionOpen.get()) {
+            try {
+                mCallbackInterface.onMessageFromNanoApp(message);
+            } catch (RemoteException e) {
+                Log.e(TAG, "RemoteException while sending message to client (host endpoint ID = "
+                        + mHostEndPointId + ")", e);
+            }
         }
     }
 }
diff --git a/services/core/java/com/android/server/location/ContextHubClientManager.java b/services/core/java/com/android/server/location/ContextHubClientManager.java
index e12686fa9016..11450092b7ba 100644
--- a/services/core/java/com/android/server/location/ContextHubClientManager.java
+++ b/services/core/java/com/android/server/location/ContextHubClientManager.java
@@ -22,6 +22,7 @@ import android.hardware.contexthub.V1_0.IContexthub;
 import android.hardware.location.IContextHubClient;
 import android.hardware.location.IContextHubClientCallback;
 import android.hardware.location.NanoAppMessage;
+import android.os.RemoteException;
 import android.util.Log;
 
 import java.util.NoSuchElementException;
@@ -88,6 +89,15 @@ import java.util.concurrent.ConcurrentHashMap;
             IContextHubClientCallback clientCallback, int contextHubId) {
         ContextHubClientBroker broker = createNewClientBroker(clientCallback, contextHubId);
 
+        try {
+            broker.attachDeathRecipient();
+        } catch (RemoteException e) {
+            // The client process has died, so we close the connection and return null.
+            Log.e(TAG, "Failed to attach death recipient to client");
+            broker.close();
+            return null;
+        }
+
         Log.d(TAG, "Registered client with host endpoint ID " + broker.getHostEndPointId());
         return IContextHubClient.Stub.asInterface(broker);
     }
@@ -121,6 +131,23 @@ import java.util.concurrent.ConcurrentHashMap;
     }
 
     /**
+     * Unregisters a client from the service.
+     *
+     * This method should be invoked as a result of a client calling the ContextHubClient.close(),
+     * or if the client process has died.
+     *
+     * @param hostEndPointId the host endpoint ID of the client that has died
+     */
+    /* package */ void unregisterClient(short hostEndPointId) {
+        if (mHostEndPointIdToClientMap.remove(hostEndPointId) != null) {
+            Log.d(TAG, "Unregistered client with host endpoint ID " + hostEndPointId);
+        } else {
+            Log.e(TAG, "Cannot unregister non-existing client with host endpoint ID "
+                    + hostEndPointId);
+        }
+    }
+
+    /**
      * Creates a new ContextHubClientBroker object for a client and registers it with the client
      * manager.
      *
@@ -142,7 +169,7 @@ import java.util.concurrent.ConcurrentHashMap;
         for (int i = 0; i <= MAX_CLIENT_ID; i++) {
             if (!mHostEndPointIdToClientMap.containsKey(id)) {
                 broker = new ContextHubClientBroker(
-                        mContext, mContextHubProxy, contextHubId, (short)id, clientCallback);
+                        mContext, mContextHubProxy, this, contextHubId, (short)id, clientCallback);
                 mHostEndPointIdToClientMap.put((short)id, broker);
                 mNextHostEndpointId = (id == MAX_CLIENT_ID) ? 0 : id + 1;
                 break;