Merge "contexthub: add 10s timeout for open request" into main

3 files changed, 179 insertions, 18 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 bbf7732c9596..7059c83d60b2 100644
--- a/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java
+++ b/services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java
@@ -44,6 +44,7 @@ import android.util.Log;
 import android.util.SparseArray;
 
 import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 
 import java.util.Collection;
 import java.util.HashSet;
@@ -53,6 +54,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 
 /**
@@ -71,6 +75,9 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
     /** The duration of wakelocks acquired during HAL callbacks */
     private static final long WAKELOCK_TIMEOUT_MILLIS = 5 * 1000;
 
+    /** The timeout of open session request */
+    @VisibleForTesting static final long OPEN_SESSION_REQUEST_TIMEOUT_SECONDS = 10;
+
     /*
      * Internal interface used to invoke client callbacks.
      */
@@ -81,6 +88,9 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
     /** The context of the service. */
     private final Context mContext;
 
+    /** The shared executor service for handling session operation timeout. */
+    private final ScheduledExecutorService mSessionTimeoutExecutor;
+
     /** The proxy to talk to the Context Hub HAL for endpoint communication. */
     private final IEndpointCommunication mHubInterface;
 
@@ -119,6 +129,8 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
 
         private SessionState mSessionState = SessionState.PENDING;
 
+        private ScheduledFuture<?> mSessionOpenTimeoutFuture;
+
         private final boolean mRemoteInitiated;
 
         /**
@@ -151,6 +163,17 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
             mSessionState = state;
         }
 
+        public void setSessionOpenTimeoutFuture(ScheduledFuture<?> future) {
+            mSessionOpenTimeoutFuture = future;
+        }
+
+        public void cancelSessionOpenTimeoutFuture() {
+            if (mSessionOpenTimeoutFuture != null) {
+                mSessionOpenTimeoutFuture.cancel(false);
+            }
+            mSessionOpenTimeoutFuture = null;
+        }
+
         public boolean isActive() {
             return mSessionState == SessionState.ACTIVE;
         }
@@ -240,7 +263,8 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
             @NonNull IContextHubEndpointCallback callback,
             String packageName,
             String attributionTag,
-            ContextHubTransactionManager transactionManager) {
+            ContextHubTransactionManager transactionManager,
+            ScheduledExecutorService sessionTimeoutExecutor) {
         mContext = context;
         mHubInterface = hubInterface;
         mEndpointManager = endpointManager;
@@ -250,6 +274,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
         mPackageName = packageName;
         mAttributionTag = attributionTag;
         mTransactionManager = transactionManager;
+        mSessionTimeoutExecutor = sessionTimeoutExecutor;
 
         mPid = Binder.getCallingPid();
         mUid = Binder.getCallingUid();
@@ -352,6 +377,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
             }
             try {
                 mHubInterface.endpointSessionOpenComplete(sessionId);
+                info.cancelSessionOpenTimeoutFuture();
                 info.setSessionState(Session.SessionState.ACTIVE);
             } catch (RemoteException | IllegalArgumentException | UnsupportedOperationException e) {
                 Log.e(TAG, "Exception while calling endpointSessionOpenComplete", e);
@@ -636,9 +662,10 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
         }
 
         // Check & handle error cases for duplicated session id.
-        final boolean existingSession;
-        final boolean existingSessionActive;
         synchronized (mOpenSessionLock) {
+            final boolean existingSession;
+            final boolean existingSessionActive;
+
             if (hasSessionId(sessionId)) {
                 existingSession = true;
                 existingSessionActive = mSessionMap.get(sessionId).isActive();
@@ -652,19 +679,23 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
             } else {
                 existingSession = false;
                 existingSessionActive = false;
-                mSessionMap.put(sessionId, new Session(initiator, true));
+                Session pendingSession = new Session(initiator, true);
+                pendingSession.setSessionOpenTimeoutFuture(
+                        mSessionTimeoutExecutor.schedule(
+                                () -> onEndpointSessionOpenRequestTimeout(sessionId),
+                                OPEN_SESSION_REQUEST_TIMEOUT_SECONDS,
+                                TimeUnit.SECONDS));
+                mSessionMap.put(sessionId, pendingSession);
             }
-        }
 
-        if (existingSession) {
-            if (existingSessionActive) {
-                // Existing session is already active, call onSessionOpenComplete.
-                openSessionRequestComplete(sessionId);
+            if (existingSession) {
+                if (existingSessionActive) {
+                    // Existing session is already active, call onSessionOpenComplete.
+                    openSessionRequestComplete(sessionId);
+                }
+                // Silence this request. The session open timeout future will handle clean up.
                 return Optional.empty();
             }
-            // Reject the session open request for now. Consider invalidating previous pending
-            // session open request based on timeout.
-            return Optional.of(Reason.OPEN_ENDPOINT_SESSION_REQUEST_REJECTED);
         }
 
         boolean success =
@@ -679,6 +710,20 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
         return success ? Optional.empty() : Optional.of(reason);
     }
 
+    private void onEndpointSessionOpenRequestTimeout(int sessionId) {
+        synchronized (mOpenSessionLock) {
+            Session s = mSessionMap.get(sessionId);
+            if (s == null || s.isActive()) {
+                return;
+            }
+            Log.w(
+                    TAG,
+                    "onEndpointSessionOpenRequestTimeout: " + "clean up session, id: " + sessionId);
+            cleanupSessionResources(sessionId);
+            mEndpointManager.halCloseEndpointSessionNoThrow(sessionId, Reason.TIMEOUT);
+        }
+    }
+
     private byte onMessageReceivedInternal(int sessionId, HubMessage message) {
         synchronized (mOpenSessionLock) {
             if (!isSessionActive(sessionId)) {
diff --git a/services/core/java/com/android/server/location/contexthub/ContextHubEndpointManager.java b/services/core/java/com/android/server/location/contexthub/ContextHubEndpointManager.java
index e1561599517d..0dc1b832f5a4 100644
--- a/services/core/java/com/android/server/location/contexthub/ContextHubEndpointManager.java
+++ b/services/core/java/com/android/server/location/contexthub/ContextHubEndpointManager.java
@@ -46,6 +46,8 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.function.Consumer;
 
 /**
@@ -112,6 +114,9 @@ import java.util.function.Consumer;
     /** The interface for endpoint communication (retrieved from HAL in init()) */
     private IEndpointCommunication mHubInterface = null;
 
+    /** Thread pool executor for handling timeout */
+    private final ScheduledExecutorService mSessionTimeoutExecutor;
+
     /*
      * The list of previous registration records.
      */
@@ -154,15 +159,31 @@ import java.util.function.Consumer;
         }
     }
 
-    /* package */ ContextHubEndpointManager(
+    @VisibleForTesting
+    ContextHubEndpointManager(
             Context context,
             IContextHubWrapper contextHubProxy,
             HubInfoRegistry hubInfoRegistry,
-            ContextHubTransactionManager transactionManager) {
+            ContextHubTransactionManager transactionManager,
+            ScheduledExecutorService scheduledExecutorService) {
         mContext = context;
         mContextHubProxy = contextHubProxy;
         mHubInfoRegistry = hubInfoRegistry;
         mTransactionManager = transactionManager;
+        mSessionTimeoutExecutor = scheduledExecutorService;
+    }
+
+    /* package */ ContextHubEndpointManager(
+            Context context,
+            IContextHubWrapper contextHubProxy,
+            HubInfoRegistry hubInfoRegistry,
+            ContextHubTransactionManager transactionManager) {
+        this(
+                context,
+                contextHubProxy,
+                hubInfoRegistry,
+                transactionManager,
+                new ScheduledThreadPoolExecutor(1));
     }
 
     /**
@@ -264,7 +285,8 @@ import java.util.function.Consumer;
                         callback,
                         packageName,
                         attributionTag,
-                        mTransactionManager);
+                        mTransactionManager,
+                        mSessionTimeoutExecutor);
         broker.register();
         mEndpointMap.put(endpointId, broker);
 
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 87cd1560509c..992c1183d0c0 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
@@ -20,6 +20,7 @@ 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.anyLong;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.never;
@@ -62,6 +63,7 @@ import org.mockito.junit.MockitoRule;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.ScheduledExecutorService;
 
 @RunWith(AndroidJUnit4.class)
 @Presubmit
@@ -97,6 +99,7 @@ public class ContextHubEndpointTest {
     private HubInfoRegistry mHubInfoRegistry;
     private ContextHubTransactionManager mTransactionManager;
     private Context mContext;
+    @Mock private ScheduledExecutorService mMockTimeoutExecutorService;
     @Mock private IEndpointCommunication mMockEndpointCommunications;
     @Mock private IContextHubWrapper mMockContextHubWrapper;
     @Mock private IContextHubEndpointCallback mMockCallback;
@@ -120,7 +123,11 @@ public class ContextHubEndpointTest {
                         mMockContextHubWrapper, mClientManager, new NanoAppStateManager());
         mEndpointManager =
                 new ContextHubEndpointManager(
-                        mContext, mMockContextHubWrapper, mHubInfoRegistry, mTransactionManager);
+                        mContext,
+                        mMockContextHubWrapper,
+                        mHubInfoRegistry,
+                        mTransactionManager,
+                        mMockTimeoutExecutorService);
         mEndpointManager.init();
     }
 
@@ -248,14 +255,20 @@ public class ContextHubEndpointTest {
                 endpoint.getAssignedHubEndpointInfo().getIdentifier(),
                 targetInfo.getIdentifier(),
                 ENDPOINT_SERVICE_DESCRIPTOR);
-
         verify(mMockCallback)
                 .onSessionOpenRequest(
                         SESSION_ID_FOR_OPEN_REQUEST, targetInfo, ENDPOINT_SERVICE_DESCRIPTOR);
 
         // Accept
         endpoint.openSessionRequestComplete(SESSION_ID_FOR_OPEN_REQUEST);
-        verify(mMockEndpointCommunications)
+
+        // Even when timeout happens, there should be no effect on this session
+        ArgumentCaptor<Runnable> runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class);
+        verify(mMockTimeoutExecutorService)
+                .schedule(runnableArgumentCaptor.capture(), anyLong(), any());
+        runnableArgumentCaptor.getValue().run();
+
+        verify(mMockEndpointCommunications, times(1))
                 .endpointSessionOpenComplete(SESSION_ID_FOR_OPEN_REQUEST);
 
         unregisterExampleEndpoint(endpoint);
@@ -331,6 +344,87 @@ public class ContextHubEndpointTest {
     }
 
     @Test
+    public void testEndpointSessionOpenRequest_rejectAfterTimeout() throws RemoteException {
+        assertThat(mEndpointManager.getNumAvailableSessions()).isEqualTo(SESSION_ID_RANGE);
+        IContextHubEndpoint endpoint = registerExampleEndpoint();
+
+        HubEndpointInfo targetInfo =
+                new HubEndpointInfo(
+                        TARGET_ENDPOINT_NAME,
+                        TARGET_ENDPOINT_ID,
+                        ENDPOINT_PACKAGE_NAME,
+                        Collections.emptyList());
+        mHubInfoRegistry.onEndpointStarted(new HubEndpointInfo[] {targetInfo});
+        mEndpointManager.onEndpointSessionOpenRequest(
+                SESSION_ID_FOR_OPEN_REQUEST,
+                endpoint.getAssignedHubEndpointInfo().getIdentifier(),
+                targetInfo.getIdentifier(),
+                ENDPOINT_SERVICE_DESCRIPTOR);
+
+        // Immediately timeout
+        ArgumentCaptor<Runnable> runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class);
+        verify(mMockTimeoutExecutorService)
+                .schedule(runnableArgumentCaptor.capture(), anyLong(), any());
+        runnableArgumentCaptor.getValue().run();
+
+        // Client's callback shouldn't matter after timeout
+        try {
+            endpoint.openSessionRequestComplete(SESSION_ID_FOR_OPEN_REQUEST);
+        } catch (IllegalArgumentException ignore) {
+            // This will throw because the session is no longer valid
+        }
+
+        // HAL will receive closeEndpointSession with Timeout as reason
+        verify(mMockEndpointCommunications, times(1))
+                .closeEndpointSession(SESSION_ID_FOR_OPEN_REQUEST, Reason.TIMEOUT);
+        // HAL will not receives open complete notifications
+        verify(mMockEndpointCommunications, never())
+                .endpointSessionOpenComplete(SESSION_ID_FOR_OPEN_REQUEST);
+
+        unregisterExampleEndpoint(endpoint);
+    }
+
+    @Test
+    public void testEndpointSessionOpenRequest_duplicatedSessionId_noopWithinTimeout()
+            throws RemoteException {
+        assertThat(mEndpointManager.getNumAvailableSessions()).isEqualTo(SESSION_ID_RANGE);
+        IContextHubEndpoint endpoint = registerExampleEndpoint();
+
+        HubEndpointInfo targetInfo =
+                new HubEndpointInfo(
+                        TARGET_ENDPOINT_NAME,
+                        TARGET_ENDPOINT_ID,
+                        ENDPOINT_PACKAGE_NAME,
+                        Collections.emptyList());
+        mHubInfoRegistry.onEndpointStarted(new HubEndpointInfo[] {targetInfo});
+        mEndpointManager.onEndpointSessionOpenRequest(
+                SESSION_ID_FOR_OPEN_REQUEST,
+                endpoint.getAssignedHubEndpointInfo().getIdentifier(),
+                targetInfo.getIdentifier(),
+                ENDPOINT_SERVICE_DESCRIPTOR);
+
+        // Duplicated session open request
+        mEndpointManager.onEndpointSessionOpenRequest(
+                SESSION_ID_FOR_OPEN_REQUEST,
+                endpoint.getAssignedHubEndpointInfo().getIdentifier(),
+                targetInfo.getIdentifier(),
+                ENDPOINT_SERVICE_DESCRIPTOR);
+
+        // Finally, endpoint approved the session open request
+        endpoint.openSessionRequestComplete(SESSION_ID_FOR_OPEN_REQUEST);
+
+        // Client API is only invoked once
+        verify(mMockCallback, times(1))
+                .onSessionOpenRequest(
+                        SESSION_ID_FOR_OPEN_REQUEST, targetInfo, ENDPOINT_SERVICE_DESCRIPTOR);
+        // HAL still receives two open complete notifications
+        verify(mMockEndpointCommunications, times(1))
+                .endpointSessionOpenComplete(SESSION_ID_FOR_OPEN_REQUEST);
+
+        unregisterExampleEndpoint(endpoint);
+    }
+
+    @Test
     public void testMessageTransaction() throws RemoteException {
         IContextHubEndpoint endpoint = registerExampleEndpoint();
         testMessageTransactionInternal(endpoint, /* deliverMessageStatus= */ true);