Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d7aa1bb3 authored by Yifei Zhang's avatar Yifei Zhang
Browse files

contexthub: add 10s timeout for open request 

- Use a shared ScheduledThreadPoolExecutor in ContextHubEndpointManager
  for all brokers.
- Remove session when timeout, and close the session with Timeout as
  reason.
- When session open request is pending, ignore duplicated open session
  request.

Test: atest ContextHubEndpointTest
Fix: 387716930
Flag: EXEMPT, bug fix
Change-Id: I3e6ba00f87d2ddd40969a81802503abb2e9a5af6
parent 37c02b7c

services/core/java/com/android/server/location/contexthub/ContextHubEndpointBroker.java

+57 −12
Original line number Diff line number Diff line
@@ -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.

        synchronized (mOpenSessionLock) {

            final boolean existingSession;

            final boolean existingSessionActive;

        synchronized (mOpenSessionLock) {



            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);

                }

                // 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)) {

services/core/java/com/android/server/location/contexthub/ContextHubEndpointManager.java

+25 −3
Original line number Diff line number Diff line
@@ -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);

services/tests/servicestests/src/com/android/server/location/contexthub/ContextHubEndpointTest.java

+97 −3
Original line number Diff line number Diff line
@@ -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);
@@ -330,6 +343,87 @@ public class ContextHubEndpointTest { 
        unregisterExampleEndpoint(endpoint);

    }



    @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();