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

Commit e80fa4e1 authored by Arthur Ishiguro's avatar Arthur Ishiguro
Browse files

Simplify Context Hub Service transaction locking

This CL simplifies the locking scheme used in the ContextHubTransactionManager to avoid granular per-transaction locks to avoid potential race conditions (for example a response and a timeout occurring at the same time, triggering duplicate calls to transaction.setResponse).

Bug: 433459445
Flag: android.chre.flags.simplify_service_transaction_lock
Test: CHQTS pass

Change-Id: I8232b1ccb153df5639186d12b3c8ff77a00ffc6b
parent e63e0b7a
Loading
Loading
Loading
Loading
+162 −65
Original line number Original line Diff line number Diff line
@@ -584,6 +584,34 @@ import java.util.concurrent.atomic.AtomicInteger;
     */
     */
    /* package */
    /* package */
    void onTransactionResponse(int transactionId, boolean success) {
    void onTransactionResponse(int transactionId, boolean success) {
        if (Flags.simplifyServiceTransactionLock()) {
            synchronized (mTransactionLock) {
                TransactionAcceptConditions conditions =
                        transaction -> {
                            if (transaction.getTransactionId() != transactionId) {
                                Log.w(
                                        TAG,
                                        "Unexpected transaction: expected "
                                                + transactionId
                                                + ", received "
                                                + transaction.getTransactionId());
                                return false;
                            }
                            return true;
                        };
                ContextHubServiceTransaction transaction = getTransactionAndHandleNext(conditions);
                if (transaction == null) {
                    Log.w(TAG, "Received unexpected transaction response");
                    return;
                }

                transaction.onTransactionComplete(
                        success
                                ? ContextHubTransaction.RESULT_SUCCESS
                                : ContextHubTransaction.RESULT_FAILED_AT_HUB);
                transaction.setComplete();
            }
        } else {
            TransactionAcceptConditions conditions =
            TransactionAcceptConditions conditions =
                    transaction -> {
                    transaction -> {
                        if (transaction.getTransactionId() != transactionId) {
                        if (transaction.getTransactionId() != transactionId) {
@@ -611,6 +639,7 @@ import java.util.concurrent.atomic.AtomicInteger;
                transaction.setComplete();
                transaction.setComplete();
            }
            }
        }
        }
    }


    /**
    /**
     * Handles a message delivery response from a Context Hub.
     * Handles a message delivery response from a Context Hub.
@@ -620,6 +649,28 @@ import java.util.concurrent.atomic.AtomicInteger;
     */
     */
    /* package */
    /* package */
    void onMessageDeliveryResponse(int messageSequenceNumber, boolean success) {
    void onMessageDeliveryResponse(int messageSequenceNumber, boolean success) {
        if (Flags.simplifyServiceTransactionLock()) {
            ContextHubServiceTransaction transaction = null;
            synchronized (mReliableMessageLock) {
                transaction = mReliableMessageTransactionMap.get(messageSequenceNumber);
                if (transaction == null) {
                    Log.w(
                            TAG,
                            "Could not find reliable message transaction with "
                                    + "message sequence number = "
                                    + messageSequenceNumber);
                    return;
                }

                removeMessageTransaction(transaction);

                completeMessageTransaction(
                        transaction,
                        success
                                ? ContextHubTransaction.RESULT_SUCCESS
                                : ContextHubTransaction.RESULT_FAILED_AT_HUB);
            }
        } else {
            ContextHubServiceTransaction transaction = null;
            ContextHubServiceTransaction transaction = null;
            synchronized (mReliableMessageLock) {
            synchronized (mReliableMessageLock) {
                transaction = mReliableMessageTransactionMap.get(messageSequenceNumber);
                transaction = mReliableMessageTransactionMap.get(messageSequenceNumber);
@@ -640,6 +691,8 @@ import java.util.concurrent.atomic.AtomicInteger;
                    success
                    success
                            ? ContextHubTransaction.RESULT_SUCCESS
                            ? ContextHubTransaction.RESULT_SUCCESS
                            : ContextHubTransaction.RESULT_FAILED_AT_HUB);
                            : ContextHubTransaction.RESULT_FAILED_AT_HUB);
        }

        mExecutor.execute(() -> processMessageTransactions());
        mExecutor.execute(() -> processMessageTransactions());
    }
    }


@@ -650,6 +703,22 @@ import java.util.concurrent.atomic.AtomicInteger;
     */
     */
    /* package */
    /* package */
    void onQueryResponse(List<NanoAppState> nanoAppStateList) {
    void onQueryResponse(List<NanoAppState> nanoAppStateList) {
        if (Flags.simplifyServiceTransactionLock()) {
            synchronized (mTransactionLock) {
                TransactionAcceptConditions conditions =
                        transaction ->
                                transaction.getTransactionType()
                                        == ContextHubTransaction.TYPE_QUERY_NANOAPPS;
                ContextHubServiceTransaction transaction = getTransactionAndHandleNext(conditions);
                if (transaction == null) {
                    Log.w(TAG, "Received unexpected query response");
                    return;
                }

                transaction.onQueryResponse(ContextHubTransaction.RESULT_SUCCESS, nanoAppStateList);
                transaction.setComplete();
            }
        } else {
            TransactionAcceptConditions conditions =
            TransactionAcceptConditions conditions =
                    transaction ->
                    transaction ->
                            transaction.getTransactionType()
                            transaction.getTransactionType()
@@ -665,6 +734,7 @@ import java.util.concurrent.atomic.AtomicInteger;
                transaction.setComplete();
                transaction.setComplete();
            }
            }
        }
        }
    }


    /** Handles a hub reset event by stopping a pending transaction and starting the next. */
    /** Handles a hub reset event by stopping a pending transaction and starting the next. */
    /* package */
    /* package */
@@ -729,9 +799,13 @@ import java.util.concurrent.atomic.AtomicInteger;
        cancelTimeoutFuture();
        cancelTimeoutFuture();


        ContextHubServiceTransaction transaction = mTransactionQueue.remove();
        ContextHubServiceTransaction transaction = mTransactionQueue.remove();
        if (Flags.simplifyServiceTransactionLock()) {
            transaction.setComplete();
        } else {
            synchronized (transaction) {
            synchronized (transaction) {
                transaction.setComplete();
                transaction.setComplete();
            }
            }
        }


        if (!mTransactionQueue.isEmpty()) {
        if (!mTransactionQueue.isEmpty()) {
            startNextTransaction();
            startNextTransaction();
@@ -766,6 +840,17 @@ import java.util.concurrent.atomic.AtomicInteger;
            if (result == ContextHubTransaction.RESULT_SUCCESS) {
            if (result == ContextHubTransaction.RESULT_SUCCESS) {
                Runnable onTimeoutFunc =
                Runnable onTimeoutFunc =
                        () -> {
                        () -> {
                            if (Flags.simplifyServiceTransactionLock()) {
                                synchronized (mTransactionLock) {
                                    if (!transaction.isComplete()) {
                                        Log.d(TAG, transaction + " timed out");
                                        transaction.onTransactionComplete(
                                                ContextHubTransaction.RESULT_FAILED_TIMEOUT);
                                        transaction.setComplete();
                                        removeTransactionAndStartNext();
                                    }
                                }
                            } else {
                                synchronized (transaction) {
                                synchronized (transaction) {
                                    if (!transaction.isComplete()) {
                                    if (!transaction.isComplete()) {
                                        Log.d(TAG, transaction + " timed out");
                                        Log.d(TAG, transaction + " timed out");
@@ -778,6 +863,7 @@ import java.util.concurrent.atomic.AtomicInteger;
                                synchronized (mTransactionLock) {
                                synchronized (mTransactionLock) {
                                    removeTransactionAndStartNext();
                                    removeTransactionAndStartNext();
                                }
                                }
                            }
                        };
                        };


                long timeoutMs = transaction.getTimeout(TimeUnit.MILLISECONDS);
                long timeoutMs = transaction.getTimeout(TimeUnit.MILLISECONDS);
@@ -787,12 +873,18 @@ import java.util.concurrent.atomic.AtomicInteger;
                } catch (Exception e) {
                } catch (Exception e) {
                    Log.e(TAG, "Error when schedule a timer", e);
                    Log.e(TAG, "Error when schedule a timer", e);
                }
                }
            } else {
                if (Flags.simplifyServiceTransactionLock()) {
                    transaction.onTransactionComplete(
                            ContextHubServiceUtil.toTransactionResult(result));
                    transaction.setComplete();
                } else {
                } else {
                    synchronized (transaction) {
                    synchronized (transaction) {
                        transaction.onTransactionComplete(
                        transaction.onTransactionComplete(
                                ContextHubServiceUtil.toTransactionResult(result));
                                ContextHubServiceUtil.toTransactionResult(result));
                        transaction.setComplete();
                        transaction.setComplete();
                    }
                    }
                }


                mTransactionQueue.remove();
                mTransactionQueue.remove();
            }
            }
@@ -874,12 +966,18 @@ import java.util.concurrent.atomic.AtomicInteger;
     * @param transaction The transaction to complete.
     * @param transaction The transaction to complete.
     * @param result The result code.
     * @param result The result code.
     */
     */
    @GuardedBy("mReliableMessageLock")
    private void completeMessageTransaction(
    private void completeMessageTransaction(
            ContextHubServiceTransaction transaction, @ContextHubTransaction.Result int result) {
            ContextHubServiceTransaction transaction, @ContextHubTransaction.Result int result) {
        if (Flags.simplifyServiceTransactionLock()) {
            transaction.onTransactionComplete(result);
            transaction.setComplete();
        } else {
            synchronized (transaction) {
            synchronized (transaction) {
                transaction.onTransactionComplete(result);
                transaction.onTransactionComplete(result);
                transaction.setComplete();
                transaction.setComplete();
            }
            }
        }


        Log.d(
        Log.d(
                TAG,
                TAG,
@@ -897,7 +995,6 @@ import java.util.concurrent.atomic.AtomicInteger;
     * @param result The result code.
     * @param result The result code.
     * @param iter The iterator for the reliable message map - used to remove the message directly.
     * @param iter The iterator for the reliable message map - used to remove the message directly.
     */
     */
    @GuardedBy("mReliableMessageLock")
    private void removeAndCompleteMessageTransaction(
    private void removeAndCompleteMessageTransaction(
            ContextHubServiceTransaction transaction,
            ContextHubServiceTransaction transaction,
            @ContextHubTransaction.Result int result,
            @ContextHubTransaction.Result int result,