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

Commit 213bebcf authored by Yifei Zhang's avatar Yifei Zhang
Browse files

contexthub: close invalid session open requests

- Move halCloseEndpointSession(NoThrow) into ContextHubEndpointManager
- Call client side onCloseEndpointSession separately
- When there's a duplicated/invalid open request, call
  closeEndpointSession as needed

Test: ContextHubEndpointTest
Flag: EXEMPT, bug fix
Bug: 387716930
Change-Id: I802e09184608341186591188d15ec943b654269e
parent 35085f4f
Loading
Loading
Loading
Loading
+42 −43
Original line number Diff line number Diff line
@@ -299,7 +299,8 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
            throw new IllegalArgumentException(
                    "Unknown session ID in closeSession: id=" + sessionId);
        }
        halCloseEndpointSession(sessionId, ContextHubServiceUtil.toHalReason(reason));
        mEndpointManager.halCloseEndpointSession(
                sessionId, ContextHubServiceUtil.toHalReason(reason));
    }

    @Override
@@ -310,7 +311,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
            // Iterate in reverse since cleanupSessionResources will remove the entry
            for (int i = mSessionMap.size() - 1; i >= 0; i--) {
                int id = mSessionMap.keyAt(i);
                halCloseEndpointSessionNoThrow(id, Reason.ENDPOINT_GONE);
                mEndpointManager.halCloseEndpointSessionNoThrow(id, Reason.ENDPOINT_GONE);
                cleanupSessionResources(id);
            }
        }
@@ -442,7 +443,8 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
                    int id = mSessionMap.keyAt(i);
                    HubEndpointInfo target = mSessionMap.get(id).getRemoteEndpointInfo();
                    if (!hasEndpointPermissions(target)) {
                        halCloseEndpointSessionNoThrow(id, Reason.PERMISSION_DENIED);
                        mEndpointManager.halCloseEndpointSessionNoThrow(
                                id, Reason.PERMISSION_DENIED);
                        onCloseEndpointSession(id, Reason.PERMISSION_DENIED);
                        // Resource cleanup is done in onCloseEndpointSession
                    }
@@ -501,17 +503,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
        mContextHubEndpointCallback.asBinder().linkToDeath(this, 0 /* flags */);
    }

    /* package */ void onEndpointSessionOpenRequest(
            int sessionId, HubEndpointInfo initiator, String serviceDescriptor) {
        Optional<Byte> error =
                onEndpointSessionOpenRequestInternal(sessionId, initiator, serviceDescriptor);
        if (error.isPresent()) {
            halCloseEndpointSessionNoThrow(sessionId, error.get());
            onCloseEndpointSession(sessionId, error.get());
            // Resource cleanup is done in onCloseEndpointSession
        }
    }

    /** Handle close endpoint callback to the client side */
    /* package */ void onCloseEndpointSession(int sessionId, byte reason) {
        if (!cleanupSessionResources(sessionId)) {
            Log.w(TAG, "Unknown session ID in onCloseEndpointSession: id=" + sessionId);
@@ -583,7 +575,7 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
        }
    }

    private Optional<Byte> onEndpointSessionOpenRequestInternal(
    /* package */ Optional<Byte> onEndpointSessionOpenRequest(
            int sessionId, HubEndpointInfo initiator, String serviceDescriptor) {
        if (!hasEndpointPermissions(initiator)) {
            Log.e(
@@ -592,23 +584,53 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
                            + initiator
                            + " doesn't have permission for "
                            + mEndpointInfo);
            return Optional.of(Reason.PERMISSION_DENIED);
            byte reason = Reason.PERMISSION_DENIED;
            onCloseEndpointSession(sessionId, reason);
            return Optional.of(reason);
        }

        // Check & handle error cases for duplicated session id.
        final boolean existingSession;
        final boolean existingSessionActive;
        synchronized (mOpenSessionLock) {
            if (hasSessionId(sessionId)) {
                Log.e(TAG, "Existing session in onEndpointSessionOpenRequest: id=" + sessionId);
                return Optional.of(Reason.UNSPECIFIED);
            }
                existingSession = true;
                existingSessionActive = mSessionMap.get(sessionId).isActive();
                Log.w(
                        TAG,
                        "onEndpointSessionOpenRequest: "
                                + "Existing session ID: "
                                + sessionId
                                + ", isActive: "
                                + existingSessionActive);
            } else {
                existingSession = false;
                existingSessionActive = false;
                mSessionMap.put(sessionId, new Session(initiator, true));
            }
        }

        if (existingSession) {
            if (existingSessionActive) {
                // Existing session is already active, call onSessionOpenComplete.
                openSessionRequestComplete(sessionId);
                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 =
                invokeCallback(
                        (consumer) ->
                                consumer.onSessionOpenRequest(
                                        sessionId, initiator, serviceDescriptor));
        return success ? Optional.empty() : Optional.of(Reason.UNSPECIFIED);
        byte reason = Reason.UNSPECIFIED;
        if (!success) {
            onCloseEndpointSession(sessionId, reason);
        }
        return success ? Optional.empty() : Optional.of(reason);
    }

    private byte onMessageReceivedInternal(int sessionId, HubMessage message) {
@@ -654,29 +676,6 @@ public class ContextHubEndpointBroker extends IContextHubEndpoint.Stub
        }
    }

    /**
     * Calls the HAL closeEndpointSession API.
     *
     * @param sessionId The session ID to close
     * @param halReason The HAL reason
     */
    private void halCloseEndpointSession(int sessionId, byte halReason) throws RemoteException {
        try {
            mHubInterface.closeEndpointSession(sessionId, halReason);
        } catch (RemoteException | IllegalArgumentException | UnsupportedOperationException e) {
            throw e;
        }
    }

    /** Same as halCloseEndpointSession but does not throw the exception */
    private void halCloseEndpointSessionNoThrow(int sessionId, byte halReason) {
        try {
            halCloseEndpointSession(sessionId, halReason);
        } catch (RemoteException | IllegalArgumentException | UnsupportedOperationException e) {
            Log.e(TAG, "Exception while calling HAL closeEndpointSession", e);
        }
    }

    /**
     * Cleans up resources related to a session with the provided ID.
     *
+56 −6
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import android.hardware.contexthub.IContextHubEndpoint;
import android.hardware.contexthub.IContextHubEndpointCallback;
import android.hardware.contexthub.IEndpointCommunication;
import android.hardware.contexthub.MessageDeliveryStatus;
import android.hardware.contexthub.Reason;
import android.os.RemoteException;
import android.os.ServiceSpecificException;
import android.util.Log;
@@ -42,6 +43,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
@@ -316,6 +318,11 @@ import java.util.function.Consumer;
        }
    }

    /** Returns if a sessionId can be allocated for the service hub. */
    private boolean isSessionIdAllocatedForService(int sessionId) {
        return sessionId > mMaxSessionId || sessionId < mMinSessionId;
    }

    /**
     * Unregisters an endpoint given its ID.
     *
@@ -337,8 +344,7 @@ import java.util.function.Consumer;
        }
    }

    @Override
    public void onEndpointSessionOpenRequest(
    private Optional<Byte> onEndpointSessionOpenRequestInternal(
            int sessionId,
            HubEndpointInfo.HubEndpointIdentifier destination,
            HubEndpointInfo.HubEndpointIdentifier initiator,
@@ -348,7 +354,7 @@ import java.util.function.Consumer;
                    TAG,
                    "onEndpointSessionOpenRequest: invalid destination hub ID: "
                            + destination.getHub());
            return;
            return Optional.of(Reason.ENDPOINT_INVALID);
        }
        ContextHubEndpointBroker broker = mEndpointMap.get(destination.getEndpoint());
        if (broker == null) {
@@ -356,7 +362,7 @@ import java.util.function.Consumer;
                    TAG,
                    "onEndpointSessionOpenRequest: unknown destination endpoint ID: "
                            + destination.getEndpoint());
            return;
            return Optional.of(Reason.ENDPOINT_INVALID);
        }
        HubEndpointInfo initiatorInfo = mHubInfoRegistry.getEndpointInfo(initiator);
        if (initiatorInfo == null) {
@@ -364,9 +370,29 @@ import java.util.function.Consumer;
                    TAG,
                    "onEndpointSessionOpenRequest: unknown initiator endpoint ID: "
                            + initiator.getEndpoint());
            return;
            return Optional.of(Reason.ENDPOINT_INVALID);
        }
        if (!isSessionIdAllocatedForService(sessionId)) {
            Log.e(
                    TAG,
                    "onEndpointSessionOpenRequest: invalid session ID, rejected:"
                            + " sessionId="
                            + sessionId);
            return Optional.of(Reason.OPEN_ENDPOINT_SESSION_REQUEST_REJECTED);
        }
        broker.onEndpointSessionOpenRequest(sessionId, initiatorInfo, serviceDescriptor);
        return broker.onEndpointSessionOpenRequest(sessionId, initiatorInfo, serviceDescriptor);
    }

    @Override
    public void onEndpointSessionOpenRequest(
            int sessionId,
            HubEndpointInfo.HubEndpointIdentifier destination,
            HubEndpointInfo.HubEndpointIdentifier initiator,
            String serviceDescriptor) {
        Optional<Byte> errorOptional =
                onEndpointSessionOpenRequestInternal(
                        sessionId, destination, initiator, serviceDescriptor);
        errorOptional.ifPresent((error) -> halCloseEndpointSessionNoThrow(sessionId, error));
    }

    @Override
@@ -418,6 +444,30 @@ import java.util.function.Consumer;
        }
    }

    /**
     * Calls the HAL closeEndpointSession API.
     *
     * @param sessionId The session ID to close
     * @param halReason The HAL reason
     */
    /* package */ void halCloseEndpointSession(int sessionId, byte halReason)
            throws RemoteException {
        try {
            mHubInterface.closeEndpointSession(sessionId, halReason);
        } catch (RemoteException | IllegalArgumentException | UnsupportedOperationException e) {
            throw e;
        }
    }

    /** Same as halCloseEndpointSession but does not throw the exception */
    /* package */ void halCloseEndpointSessionNoThrow(int sessionId, byte halReason) {
        try {
            halCloseEndpointSession(sessionId, halReason);
        } catch (RemoteException | IllegalArgumentException | UnsupportedOperationException e) {
            Log.e(TAG, "Exception while calling HAL closeEndpointSession", e);
        }
    }

    @Override
    public String toString() {
        StringBuilder sb = new StringBuilder();
+103 −0
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.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -67,12 +68,15 @@ public class ContextHubEndpointTest {
    private static final int SESSION_ID_RANGE = ContextHubEndpointManager.SERVICE_SESSION_RANGE;
    private static final int MIN_SESSION_ID = 0;
    private static final int MAX_SESSION_ID = MIN_SESSION_ID + SESSION_ID_RANGE - 1;
    private static final int SESSION_ID_FOR_OPEN_REQUEST = MAX_SESSION_ID + 1;
    private static final int INVALID_SESSION_ID_FOR_OPEN_REQUEST = MIN_SESSION_ID + 1;

    private static final String ENDPOINT_NAME = "Example test endpoint";
    private static final int ENDPOINT_ID = 1;
    private static final String ENDPOINT_PACKAGE_NAME = "com.android.server.location.contexthub";

    private static final String TARGET_ENDPOINT_NAME = "Example target endpoint";
    private static final String ENDPOINT_SERVICE_DESCRIPTOR = "serviceDescriptor";
    private static final int TARGET_ENDPOINT_ID = 1;

    private static final int SAMPLE_MESSAGE_TYPE = 1234;
@@ -220,6 +224,105 @@ public class ContextHubEndpointTest {
        assertThat(mEndpointManager.getNumAvailableSessions()).isEqualTo(SESSION_ID_RANGE);
    }

    @Test
    public void testEndpointSessionOpenRequest() 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);

        verify(mMockCallback)
                .onSessionOpenRequest(
                        SESSION_ID_FOR_OPEN_REQUEST, targetInfo, ENDPOINT_SERVICE_DESCRIPTOR);

        // Accept
        endpoint.openSessionRequestComplete(SESSION_ID_FOR_OPEN_REQUEST);
        verify(mMockEndpointCommunications)
                .endpointSessionOpenComplete(SESSION_ID_FOR_OPEN_REQUEST);

        unregisterExampleEndpoint(endpoint);
    }

    @Test
    public void testEndpointSessionOpenRequestWithInvalidSessionId() 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(
                INVALID_SESSION_ID_FOR_OPEN_REQUEST,
                endpoint.getAssignedHubEndpointInfo().getIdentifier(),
                targetInfo.getIdentifier(),
                ENDPOINT_SERVICE_DESCRIPTOR);
        verify(mMockEndpointCommunications)
                .closeEndpointSession(
                        INVALID_SESSION_ID_FOR_OPEN_REQUEST,
                        Reason.OPEN_ENDPOINT_SESSION_REQUEST_REJECTED);
        verify(mMockCallback, never())
                .onSessionOpenRequest(
                        INVALID_SESSION_ID_FOR_OPEN_REQUEST,
                        targetInfo,
                        ENDPOINT_SERVICE_DESCRIPTOR);

        unregisterExampleEndpoint(endpoint);
    }

    @Test
    public void testEndpointSessionOpenRequest_duplicatedSessionId_noopWhenSessionIsActive()
            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);
        endpoint.openSessionRequestComplete(SESSION_ID_FOR_OPEN_REQUEST);
        // Now session with id SESSION_ID_FOR_OPEN_REQUEST is active

        // Duplicated session open request
        mEndpointManager.onEndpointSessionOpenRequest(
                SESSION_ID_FOR_OPEN_REQUEST,
                endpoint.getAssignedHubEndpointInfo().getIdentifier(),
                targetInfo.getIdentifier(),
                ENDPOINT_SERVICE_DESCRIPTOR);

        // 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(2))
                .endpointSessionOpenComplete(SESSION_ID_FOR_OPEN_REQUEST);

        unregisterExampleEndpoint(endpoint);
    }

    @Test
    public void testMessageTransaction() throws RemoteException {
        IContextHubEndpoint endpoint = registerExampleEndpoint();