Merge "Close session on message send failure" into main

2 files changed, 107 insertions, 8 deletions

diff --git a/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java b/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java
index ccb9e3ea5cbe..41fd29da64f1 100644
--- a/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java
+++ b/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java
@@ -33,6 +33,7 @@ import android.hardware.contexthub.MessageDeliveryStatus;
 import android.hardware.contexthub.Reason;
 import android.hardware.location.ContextHubTransaction;
 import android.hardware.location.IContextHubTransactionCallback;
+import android.hardware.location.NanoAppState;
 import android.os.Binder;
 import android.os.IBinder;
 import android.os.PowerManager;
@@ -48,6 +49,7 @@ import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -276,6 +278,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
 
         int sessionId = mEndpointManager.reserveSessionId();
         EndpointInfo halEndpointInfo = ContextHubServiceUtil.convertHalEndpointInfo(destination);
+        Log.d(TAG, "openSession: sessionId=" + sessionId);
 
         synchronized (mOpenSessionLock) {
             try {
@@ -301,6 +304,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
             throw new IllegalArgumentException(
                     "Unknown session ID in closeSession: id=" + sessionId);
         }
+        Log.d(TAG, "closeSession: sessionId=" + sessionId + " reason=" + reason);
         mEndpointManager.halCloseEndpointSession(
                 sessionId, ContextHubServiceUtil.toHalReason(reason));
     }
@@ -373,12 +377,43 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
                 try {
                     mHubInterface.sendMessageToEndpoint(sessionId, halMessage);
                 } catch (RemoteException e) {
-                    Log.w(TAG, "Exception while sending message on session " + sessionId, e);
+                    Log.e(
+                            TAG,
+                            "Exception while sending message on session "
+                                    + sessionId
+                                    + ", closing session",
+                            e);
+                    notifySessionClosedToBoth(sessionId, Reason.UNSPECIFIED);
                 }
             } else {
+                IContextHubTransactionCallback wrappedCallback =
+                        new IContextHubTransactionCallback.Stub() {
+                            @Override
+                            public void onQueryResponse(int result, List<NanoAppState> appStates)
+                                    throws RemoteException {
+                                Log.w(TAG, "Unexpected onQueryResponse callback");
+                            }
+
+                            @Override
+                            public void onTransactionComplete(int result) throws RemoteException {
+                                callback.onTransactionComplete(result);
+                                if (result != ContextHubTransaction.RESULT_SUCCESS) {
+                                    Log.e(
+                                            TAG,
+                                            "Failed to send reliable message "
+                                                    + message
+                                                    + ", closing session");
+                                    notifySessionClosedToBoth(sessionId, Reason.UNSPECIFIED);
+                                }
+                            }
+                        };
                 ContextHubServiceTransaction transaction =
                         mTransactionManager.createSessionMessageTransaction(
-                                mHubInterface, sessionId, halMessage, mPackageName, callback);
+                                mHubInterface,
+                                sessionId,
+                                halMessage,
+                                mPackageName,
+                                wrappedCallback);
                 try {
                     mTransactionManager.addTransaction(transaction);
                     info.setReliableMessagePending(transaction.getMessageSequenceNumber());
@@ -445,10 +480,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
                     int id = mSessionMap.keyAt(i);
                     HubEndpointInfo target = mSessionMap.get(id).getRemoteEndpointInfo();
                     if (!hasEndpointPermissions(target)) {
-                        mEndpointManager.halCloseEndpointSessionNoThrow(
-                                id, Reason.PERMISSION_DENIED);
-                        onCloseEndpointSession(id, Reason.PERMISSION_DENIED);
-                        // Resource cleanup is done in onCloseEndpointSession
+                        notifySessionClosedToBoth(id, Reason.PERMISSION_DENIED);
                     }
                 }
             }
@@ -532,8 +564,17 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
 
     /* package */ void onMessageReceived(int sessionId, HubMessage message) {
         byte errorCode = onMessageReceivedInternal(sessionId, message);
-        if (errorCode != ErrorCode.OK && message.isResponseRequired()) {
-            sendMessageDeliveryStatus(sessionId, message.getMessageSequenceNumber(), errorCode);
+        if (errorCode != ErrorCode.OK) {
+            Log.e(TAG, "Failed to send message to endpoint: " + message + ", closing session");
+            if (message.isResponseRequired()) {
+                sendMessageDeliveryStatus(sessionId, message.getMessageSequenceNumber(), errorCode);
+            } else {
+                notifySessionClosedToBoth(
+                        sessionId,
+                        (errorCode == ErrorCode.PERMISSION_DENIED)
+                                ? Reason.PERMISSION_DENIED
+                                : Reason.UNSPECIFIED);
+            }
         }
     }
 
@@ -800,4 +841,16 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
                         + "-0x"
                         + Long.toHexString(endpoint.getIdentifier().getEndpoint()));
     }
+
+    /**
+     * Notifies to both the HAL and the app that a session has been closed.
+     *
+     * @param sessionId The ID of the session that was closed
+     * @param halReason The HAL reason for closing the session
+     */
+    private void notifySessionClosedToBoth(int sessionId, byte halReason) {
+        Log.d(TAG, "notifySessionClosedToBoth: sessionId=" + sessionId + ", reason=" + halReason);
+        mEndpointManager.halCloseEndpointSessionNoThrow(sessionId, halReason);
+        onCloseEndpointSession(sessionId, halReason);
+    }
 }
diff --git a/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java b/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java
index 43b1ec393bf6..87cd1560509c 100644
--- a/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java
+++ b/services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java
@@ -19,7 +19,9 @@ package com.android.server.location.contexthub;
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.times;
@@ -29,6 +31,7 @@ import static org.mockito.Mockito.when;
 import android.content.Context;
 import android.hardware.contexthub.EndpointInfo;
 import android.hardware.contexthub.ErrorCode;
+import android.hardware.contexthub.HubEndpoint;
 import android.hardware.contexthub.HubEndpointInfo;
 import android.hardware.contexthub.HubEndpointInfo.HubEndpointIdentifier;
 import android.hardware.contexthub.HubMessage;
@@ -385,6 +388,49 @@ public class ContextHubEndpointTest {
         unregisterExampleEndpoint(endpoint);
     }
 
+    @Test
+    public void testUnreliableMessageFailureClosesSession() throws RemoteException {
+        IContextHubEndpoint endpoint = registerExampleEndpoint();
+        int sessionId = openTestSession(endpoint);
+
+        doThrow(new RemoteException("Intended exception in test"))
+                .when(mMockCallback)
+                .onMessageReceived(anyInt(), any(HubMessage.class));
+        mEndpointManager.onMessageReceived(sessionId, SAMPLE_UNRELIABLE_MESSAGE);
+        ArgumentCaptor<HubMessage> messageCaptor = ArgumentCaptor.forClass(HubMessage.class);
+        verify(mMockCallback).onMessageReceived(eq(sessionId), messageCaptor.capture());
+        assertThat(messageCaptor.getValue()).isEqualTo(SAMPLE_UNRELIABLE_MESSAGE);
+
+        verify(mMockEndpointCommunications).closeEndpointSession(sessionId, Reason.UNSPECIFIED);
+        verify(mMockCallback).onSessionClosed(sessionId, HubEndpoint.REASON_FAILURE);
+        assertThat(mEndpointManager.getNumAvailableSessions()).isEqualTo(SESSION_ID_RANGE);
+
+        unregisterExampleEndpoint(endpoint);
+    }
+
+    @Test
+    public void testSendUnreliableMessageFailureClosesSession() throws RemoteException {
+        IContextHubEndpoint endpoint = registerExampleEndpoint();
+        int sessionId = openTestSession(endpoint);
+
+        doThrow(new RemoteException("Intended exception in test"))
+                .when(mMockEndpointCommunications)
+                .sendMessageToEndpoint(anyInt(), any(Message.class));
+        endpoint.sendMessage(sessionId, SAMPLE_UNRELIABLE_MESSAGE, /* callback= */ null);
+        ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);
+        verify(mMockEndpointCommunications)
+                .sendMessageToEndpoint(eq(sessionId), messageCaptor.capture());
+        Message message = messageCaptor.getValue();
+        assertThat(message.type).isEqualTo(SAMPLE_UNRELIABLE_MESSAGE.getMessageType());
+        assertThat(message.content).isEqualTo(SAMPLE_UNRELIABLE_MESSAGE.getMessageBody());
+
+        verify(mMockEndpointCommunications).closeEndpointSession(sessionId, Reason.UNSPECIFIED);
+        verify(mMockCallback).onSessionClosed(sessionId, HubEndpoint.REASON_FAILURE);
+        assertThat(mEndpointManager.getNumAvailableSessions()).isEqualTo(SESSION_ID_RANGE);
+
+        unregisterExampleEndpoint(endpoint);
+    }
+
     /** A helper method to create a session and validates reliable message sending. */
     private void testMessageTransactionInternal(
             IContextHubEndpoint endpoint, boolean deliverMessageStatus) throws RemoteException {