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

Commit 52450d62 authored by Thomas Stuart's avatar Thomas Stuart
Browse files

replace Ringer reference to <Handler> with a <Executor>

This change refactors `Ringer.java` to replace its internal `Handler`
and `HandlerThread` with a `java.util.concurrent.ExecutorService`
(specifically, a single-thread executor). This aligns with the
recommended practice of using the `java.util.concurrent` framework
for asynchronous operations in Mainline modules.

Key modifications include:
- Replaced `Ringer.mHandler` and its associated `HandlerThread` with
  `mRingerExecutor` (an `ExecutorService`).
- Calls to `handler.post()` are now `executor.execute()`.
- Introduced `LoggedExecutor` to adapt the existing session-based
  logging mechanism (previously in `LoggedHandlerExecutor`) to the new
  `ExecutorService`, preserving detailed logging capabilities.
- Added `Ringer.shutdownExecutor()` for proper lifecycle management of
  the `ExecutorService`.

This refactoring improves the stability, reduces platform coupling, and
enhances the Mainline compatibility of the Ringer component.

Flag: com.android.server.telecom.flags.resolve_hidden_dependencies_two
Bug: 308452386
Test: atest com.android.server.telecom.tests.RingerTest
Change-Id: I3abd109eee162fe767761650fedcbf8687a54ee9
parent 3e07387d
Loading
Loading
Loading
Loading
+61 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2025 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.android.server.telecom;

import android.telecom.Logging.Runnable;

import java.util.concurrent.Executor;

/** An executor that starts a log session before executing a runnable */
public class LoggedExecutor implements Executor {
    private final Executor mDelegateExecutor;
    private final String mSessionName;
    private final Object mLock;

    /**
     * Creates a LoggedExecutor that wraps another executor to provide session-based logging
     * for submitted tasks using Telecom's custom Runnable.
     *
     * @param delegateExecutor The actual executor that will run the tasks.
     * @param sessionName The name for the logging subsession.
     * @param lock The synchronization lock. If null, the custom Runnable will create its own.
     */
    public LoggedExecutor(Executor delegateExecutor, String sessionName, Object lock) {
        this.mDelegateExecutor = delegateExecutor;
        this.mSessionName = sessionName;
        this.mLock = lock;
    }

    @Override
    public void execute(java.lang.Runnable command) {
        Runnable telecomSessionRunnable = new Runnable(mSessionName, mLock) {
            @Override
            public void loggedRun() {
                command.run();
            }
        };

        // telecomSessionRunnable.prepare:
        //      a. Calls Log.createSubsession() and stores the session.
        //      b. Returns the *inner* mRunnable (a standard java.lang.Runnable).
        java.lang.Runnable preparedInnerRunnable = telecomSessionRunnable.prepare();

        // Submit this 'preparedInnerRunnable' to the mDelegateExecutor.
        //    The run() method of 'preparedInnerRunnable' will handle continuing the session,
        //    calling the loggedRun() we defined above, and then ending the session.
        mDelegateExecutor.execute(preparedInnerRunnable);
    }
}
 No newline at end of file
+82 −6
Original line number Diff line number Diff line
@@ -64,6 +64,9 @@ import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.function.BiConsumer;
@@ -245,6 +248,22 @@ public class Ringer {
     */
    private final Object mLock;

    /**
     * Manages a dedicated single background thread for executing Ringer-specific tasks
     * asynchronously, such as calculating ringer attributes and posting accessibility updates.
     * <p>
     * This ExecutorService replaces the previous Handler/HandlerThread mechanism to align with
     * Mainline module guidelines, which restrict direct Handler usage to reduce platform coupling
     * and enhance stability.
     */
    private ExecutorService mRingerExecutor = null;
    /**
     * Guards mRingerExecutor's lifecycle (lazy initialization, shutdown). This prevents
     * race conditions during init that could lead to multiple executor instances & resource leaks.
     * Kept separate from mLock (ringer operational state) to minimize unrelated contention.
     */
    private final Object mExecutorLock = new Object();

    /** Initializes the Ringer. */
    @VisibleForTesting
    public Ringer(
@@ -289,6 +308,24 @@ public class Ringer {
                com.android.internal.R.bool.config_ringtoneVibrationSettingsSupported);
    }

    public void shutdownExecutor() {
        Log.i(this, "Shutting down Ringer executor.");
        synchronized (mExecutorLock) {
            if (mRingerExecutor != null) {
                mRingerExecutor.shutdown();
                try {
                    if (!mRingerExecutor.awaitTermination(500, TimeUnit.MILLISECONDS)) {
                        mRingerExecutor.shutdownNow();
                    }
                } catch (InterruptedException e) {
                    mRingerExecutor.shutdownNow();
                    Thread.currentThread().interrupt();
                }
                mRingerExecutor = null;
            }
        }
    }

    @VisibleForTesting
    public void setBlockOnRingingFuture(CompletableFuture<Void> future) {
        mBlockOnRingingFuture = future;
@@ -324,7 +361,7 @@ public class Ringer {
            // TODO: moving these RingerAttributes calculation out of Telecom lock to avoid blocking
            CompletableFuture<RingerAttributes> ringerAttributesFuture = CompletableFuture
                    .supplyAsync(() -> getRingerAttributes(foregroundCall, isHfpDeviceAttached),
                            new LoggedHandlerExecutor(getHandler(), "R.sR", null));
                            getLoggedExecutor("R.sR"));

            RingerAttributes attributes = null;
            try {
@@ -367,10 +404,16 @@ public class Ringer {
            final boolean shouldFlash = attributes.shouldRingForContact();
            if (mAccessibilityManagerAdapter != null && shouldFlash) {
                Log.addEvent(foregroundCall, LogUtils.Events.FLASH_NOTIFICATION_START);
                if (mFlags.resolveHiddenDependenciesTwo()) {
                    getExecutor().execute(() ->
                            mAccessibilityManagerAdapter.startFlashNotificationSequence(mContext,
                                    AccessibilityManager.FLASH_REASON_CALL));
                } else {
                    getHandler().post(() ->
                            mAccessibilityManagerAdapter.startFlashNotificationSequence(mContext,
                                    AccessibilityManager.FLASH_REASON_CALL));
                }
            }

            // Determine if the settings and DND mode indicate that the vibrator can be used right
            // now.
@@ -665,9 +708,14 @@ public class Ringer {
        final Call foregroundCall = mRingingCall != null ? mRingingCall : mVibratingCall;
        if (mAccessibilityManagerAdapter != null) {
            Log.addEvent(foregroundCall, LogUtils.Events.FLASH_NOTIFICATION_STOP);
            if (mFlags.resolveHiddenDependenciesTwo()) {
                getExecutor().execute(() ->
                        mAccessibilityManagerAdapter.stopFlashNotificationSequence(mContext));
            } else {
                getHandler().post(() ->
                        mAccessibilityManagerAdapter.stopFlashNotificationSequence(mContext));
            }
        }

        synchronized (mLock) {
            if (mRingingCall != null) {
@@ -908,6 +956,34 @@ public class Ringer {
        return mHandler;
    }

    private java.util.concurrent.Executor getLoggedExecutor(String functionName) {
        if (mFlags.resolveHiddenDependenciesTwo()) {
            return new LoggedExecutor(getExecutor(), functionName, null);
        } else {
            return new LoggedHandlerExecutor(getHandler(), functionName, null);
        }
    }

    public ExecutorService getExecutor() {
        synchronized (mExecutorLock) {
            if (mRingerExecutor == null) {
                ThreadFactory ringerThread = new ThreadFactory() {
                    @Override
                    public Thread newThread(Runnable r) {
                        Thread t = new Thread(r, "Ringer-Executor");
                        return t;
                    }
                };
                // A single-thread executor is chosen here to ensure that Ringer-internal
                // background tasks (e.g., ringer attribute calculation, accessibility updates)
                // are processed sequentially. This mirrors the execution model of the previous
                // Handler/HandlerThread, preserving task order and predictable behavior.
                mRingerExecutor = Executors.newSingleThreadExecutor(ringerThread);
            }
        }
        return mRingerExecutor;
    }

    @VisibleForTesting
    public boolean waitForAttributesCompletion() throws InterruptedException {
        if (mAttributesLatch != null) {
+67 −0
Original line number Diff line number Diff line
@@ -22,7 +22,10 @@ import static android.provider.Settings.Global.ZEN_MODE_IMPORTANT_INTERRUPTIONS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assume.assumeNotNull;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -96,6 +99,10 @@ import org.mockito.Spy;

import java.time.Duration;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;

@RunWith(JUnit4.class)
@@ -206,6 +213,9 @@ public class RingerTest extends TelecomTestCase {
    @Override
    @After
    public void tearDown() throws Exception {
        if (mRingerUnderTest != null) {
            mRingerUnderTest.shutdownExecutor();
        }
        super.tearDown();
    }

@@ -965,6 +975,63 @@ public class RingerTest extends TelecomTestCase {
        }
    }

    /**
     * Verifies that the Ringer's internal ExecutorService is correctly shut down via
     * {@link Ringer#shutdownExecutor()} and can be re-initialized on a subsequent call to
     * {@link Ringer#getExecutor()}.
     */
    @SmallTest
    @Test
    public void testExecutorServiceLifecycle_ShutdownAndRecreation() throws Exception {
        // Get the initial executor and verify it's active
        ExecutorService executor1 = mRingerUnderTest.getExecutor();
        assertNotNull(executor1);
        assertFalse("ExecutorService should not be shutdown initially", executor1.isShutdown());

        // Ensure it can execute a task
        CountDownLatch task1Latch = new CountDownLatch(1);
        executor1.execute(task1Latch::countDown);
        assertTrue("Task 1 should execute on initial executor",
                task1Latch.await(1, TimeUnit.SECONDS));

        // Shutdown the executor
        mRingerUnderTest.shutdownExecutor();

        // Verify the original executor instance is shutdown
        assertTrue("ExecutorService (exec1) should be shutdown after call to shutdownExecutor()",
                executor1.isShutdown());
        // Depending on the exact implementation of shutdownExecutor,
        // isTerminated might also be true or become true quickly.
        // shutdownExecutor calls awaitTermination, so it should be terminated.
        assertTrue("ExecutorService (exec1) should be terminated",
                executor1.awaitTermination(500, TimeUnit.MILLISECONDS));


        // Attempting to execute on the old, shutdown executor should fail
        try {
            executor1.execute(() -> {
                // This should not run
            });
            fail("Executing a task on a shutdown executor (exec1) should throw" +
                    " RejectedExecutionException");
        } catch (RejectedExecutionException e) {
            // Expected behavior
        }

        // Get the executor again; it should be a new, active instance
        ExecutorService executor2 = mRingerUnderTest.getExecutor();
        assertNotNull(executor2);
        assertNotSame("Should get a new ExecutorService instance after shutdown and re-request",
                executor1, executor2);
        assertFalse("New ExecutorService (exec2) should not be shutdown", executor2.isShutdown());

        // Ensure the new executor can execute tasks
        CountDownLatch task2Latch = new CountDownLatch(1);
        executor2.execute(task2Latch::countDown);
        assertTrue("Task 2 should execute on new executor (exec2)",
                task2Latch.await(1, TimeUnit.SECONDS));
    }

    /**
     * Call startRinging and wait for its effects to have played out, to allow reliable assertions
     * after it. The effects are generally "start playing ringtone" and "start vibration" - not