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

Commit c70e8988 authored by Thomas Stuart's avatar Thomas Stuart
Browse files

fix VoipCallTransaction MessageQueue log spam

Background Context. There has been a few instances of Bug Reports
referencing a MessageQueue IllegalStateException in the
VoipCallTransaction class as the reason for the bug.  Every time,
the issue turned out to be something else but the log spam wasted
resources and time following a false lead.

Issue. The ExceptionallyAsync block was using the same Executor that the
thenApplyAsync was using.  The thenApplyAsync block always quits the
Handler and the ExceptionallyAsync task appears to post a message on the
dead Handler.  Also, if an exception is encountered, the
TransactionManager should be notified right away instead of waiting for
a timeout.

Fix.
- use .exceptionally instead of .exceptionallyAsync
- notify other transactions and the transaction manager immmediately if
  the future hits an exception.

Flag: NONE <fixes log spam. should make V>
Fixes: 317070482
Test: atest com.android.server.telecom.tests.VoipCallTransactionTest
      && looking through the logs when running a transactional test
         to verify log spam is gone.
Change-Id: I8d7b8dac20dce5b4cfffd65a7998271fc6e15ee7
parent 1307e792
Loading
Loading
Loading
Loading
+21 −10
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.server.telecom.voip;

import android.os.Handler;
import android.os.HandlerThread;
import android.telecom.CallException;
import android.telecom.Log;

import com.android.internal.annotations.VisibleForTesting;
@@ -195,21 +196,31 @@ public class VoipCallTransaction {

    protected final void scheduleTransaction() {
        LoggedHandlerExecutor executor = new LoggedHandlerExecutor(mHandler,
                mTransactionName + "@" + hashCode() + ".pT", mLock);
                mTransactionName + "@" + hashCode() + ".sT", mLock);
        CompletableFuture<Void> future = CompletableFuture.completedFuture(null);
        future.thenComposeAsync(this::processTransaction, executor)
                .thenApplyAsync((Function<VoipCallTransactionResult, Void>) result -> {
                    notifyListenersOfResult(result);
                    return null;
                }, executor)
                .exceptionally((throwable -> {
                    // Do NOT wait for the timeout in order to finish this failed transaction.
                    // Instead, propagate the failure to the other transactions immediately!
                    String errorMessage = throwable != null ? throwable.getMessage() :
                            "encountered an exception while processing " + mTransactionName;
                    notifyListenersOfResult(new VoipCallTransactionResult(
                            CallException.CODE_ERROR_UNKNOWN, errorMessage));
                    Log.e(this, throwable, "Error while executing transaction.");
                    return null;
                }));
    }

    protected void notifyListenersOfResult(VoipCallTransactionResult result){
        mCompleted.set(true);
        finish(result);
        if (mCompleteListener != null) {
            mCompleteListener.onTransactionCompleted(result, mTransactionName);
        }
                    return null;
                    }, executor)
                .exceptionallyAsync((throwable -> {
                    Log.e(this, throwable, "Error while executing transaction.");
                    return null;
                }), executor);
    }

    protected CompletionStage<VoipCallTransactionResult> processTransaction(Void v) {
@@ -248,7 +259,7 @@ public class VoipCallTransaction {
        if (mSubTransactions != null && !mSubTransactions.isEmpty()) {
            mSubTransactions.forEach( t -> t.finish(isTimedOut, result));
        }
        mHandlerThread.quit();
        mHandlerThread.quitSafely();
    }

    /**
+34 −0
Original line number Diff line number Diff line
@@ -303,6 +303,40 @@ public class VoipCallTransactionTest extends TelecomTestCase {
        verifyTransactionsFinished(t1, t2);
    }

    /**
     * This test verifies that if a transaction encounters an exception while processing it,
     * the exception finishes the transaction immediately instead of waiting for the timeout.
     */
    @SmallTest
    @Test
    public void testTransactionHitsException()
            throws ExecutionException, InterruptedException, TimeoutException {
        // GIVEN - a transaction that throws an exception when processing
        TestVoipCallTransaction t1 = new TestVoipCallTransaction(
                "t1",
                100L,
                TestVoipCallTransaction.EXCEPTION);
        // verify the TransactionManager informs the client of the failed transaction
        CompletableFuture<String> exceptionFuture = new CompletableFuture<>();
        OutcomeReceiver<VoipCallTransactionResult, CallException> outcomeExceptionReceiver =
                new OutcomeReceiver<>() {
                    @Override
                    public void onResult(VoipCallTransactionResult result) {
                    }

                    @Override
                    public void onError(CallException e) {
                        exceptionFuture.complete(e.getMessage());
                    }
                };
        // WHEN - add and process the transaction
        mTransactionManager.addTransaction(t1, outcomeExceptionReceiver);
        exceptionFuture.get(200L, TimeUnit.MILLISECONDS);
        // THEN - assert the transaction finished and failed
        assertTrue(mLog.toString().contains("t1 exception;\n"));
        verifyTransactionsFinished(t1);
    }

    @SmallTest
    @Test
    public void testTransactionResultException()