From 49e030fbd2ce6e4e12d2c468f0ef3d329a54fca0 Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Thu, 16 Nov 2017 11:54:42 -0800 Subject: Implements client close and death notification Bug: 67734082 Test: make from root Change-Id: Ia139fd6d4bb04c569a9ee3672e21e2700daa40a9 --- .../hardware/location/ContextHubClient.java | 32 +++++++- .../hardware/location/IContextHubClient.aidl | 3 + .../server/location/ContextHubClientBroker.java | 88 ++++++++++++++++++---- .../server/location/ContextHubClientManager.java | 29 ++++++- 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"; /* @@ -45,6 +51,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. */ @@ -60,16 +71,31 @@ 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. * @@ -80,20 +106,48 @@ 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 */ @@ -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); } @@ -120,6 +130,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; -- cgit v1.2.3-59-g8ed1b