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

Commit aa53b661 authored by Atneya Nair's avatar Atneya Nair
Browse files

Prevent Thread Leaks in STMiddleware

We create threads which are never quit in several utilities in
SoundTriggerMiddleware leading to performance issues.

Appropriately gate their life time via detach.

Bug: 236788273
Test: atest UptimeTimerTest.java
Test: atest SoundTriggerHalConcurrentCaptureHandlerTest.java
Test: Manual Logging verification
Change-Id: I478825ef50c9d6d539ecc93ad0bab16092f17152
parent d50b929a
Loading
Loading
Loading
Loading
+51 −17
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@
package com.android.server.soundtrigger_middleware;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.media.soundtrigger.ModelParameterRange;
import android.media.soundtrigger.PhraseRecognitionEvent;
import android.media.soundtrigger.PhraseSoundModel;
@@ -289,21 +290,35 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal
     * they were pushed.
     * <li>Events can be flushed via {@link #flush()}. This will block until all events pushed prior
     * to this call have been fully processed.
     * TODO(b/246584464) Remove and replace with Handler (and other concurrency fixes).
     * </ul>
     */
    private static class CallbackThread {
    private static class CallbackThread implements Runnable {
        private final Queue<Runnable> mList = new LinkedList<>();
        private int mPushCount = 0;
        private int mProcessedCount = 0;

        private boolean mQuitting = false;
        private final Thread mThread;
        /**
         * Ctor. Starts the thread.
         */
        CallbackThread() {
            new Thread(() -> {
            mThread = new Thread(this , "STHAL Concurrent Capture Handler Callback");
            mThread.start();
        }
        /**
         * Consume items in the queue until quit is called.
         */
        public void run() {
            try {
                while (true) {
                        pop().run();
                    Runnable toRun = pop();
                    if (toRun == null) {
                      // There are no longer any runnables to run,
                      // and quit() has been called.
                      return;
                    }
                    toRun.run();
                    synchronized (mList) {
                        mProcessedCount++;
                        mList.notifyAll();
@@ -311,21 +326,24 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal
                }
            } catch (InterruptedException e) {
                // If interrupted, exit.
                // Note, this is dangerous wrt to flush.
            }
            }).start();
        }

        /**
         * Push a new runnable to the queue, with no deduping.
         * If quit has been called, the runnable will not be pushed.
         *
         * @param runnable The runnable to push.
         * @return If the runnable was successfully pushed.
         */
        void push(Runnable runnable) {
        boolean push(Runnable runnable) {
            synchronized (mList) {
                if (mQuitting) return false;
                mList.add(runnable);
                mPushCount++;
                mList.notifyAll();
            }
            return true;
        }

        /**
@@ -343,11 +361,26 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal
            }
        }

        private Runnable pop() throws InterruptedException {
        /**
         * Quit processing after the queue is cleared.
         * All subsequent calls to push will fail.
         * Note, this does not flush.
         */
        void quit() {
            synchronized(mList) {
                mQuitting = true;
                mList.notifyAll();
            }
        }

        // Returns the next runnable when available.
        // Returns null iff the list is empty and quit has been called.
        private @Nullable Runnable pop() throws InterruptedException {
            synchronized (mList) {
                while (mList.isEmpty()) {
                while (mList.isEmpty() && !mQuitting) {
                    mList.wait();
                }
                if (mList.isEmpty() && mQuitting) return null;
                return mList.remove();
            }
        }
@@ -372,6 +405,7 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal
    public void detach() {
        mDelegate.detach();
        mNotifier.unregisterListener(this);
        mCallbackThread.quit();
    }

    ////////////////////////////////////////////////////////////////////////////////////////////////
+2 −0
Original line number Diff line number Diff line
@@ -150,6 +150,8 @@ public class SoundTriggerHalWatchdog implements ISoundTriggerHal {
    @Override
    public void detach() {
        mUnderlying.detach();
        // We should no longer have any pending calls
        mTimer.quit();
    }

    private class Watchdog implements AutoCloseable {
+22 −37
Original line number Diff line number Diff line
@@ -18,9 +18,7 @@ package com.android.server.soundtrigger_middleware;

import android.annotation.NonNull;
import android.os.Handler;
import android.os.Looper;

import java.util.concurrent.atomic.AtomicReference;
import android.os.HandlerThread;

/**
 * A simple timer, similar to java.util.Timer, but using the "uptime clock".
@@ -33,58 +31,45 @@ import java.util.concurrent.atomic.AtomicReference;
 * task.cancel();
 */
class UptimeTimer {
    private Handler mHandler = null;
    private final Handler mHandler;
    private final HandlerThread mHandlerThread;

    interface Task {
        void cancel();
    }

    UptimeTimer(String threadName) {
        new Thread(this::threadFunc, threadName).start();
        synchronized (this) {
            while (mHandler == null) {
                try {
                    wait();
                } catch (InterruptedException e) {
                    throw new RuntimeException(e);
                }
            }
        }
        mHandlerThread = new HandlerThread(threadName);
        mHandlerThread.start();
        // Blocks until looper init
        mHandler = new Handler(mHandlerThread.getLooper());
    }

    // Note, this method is not internally synchronized.
    // This is safe since Handlers are internally synchronized.
    Task createTask(@NonNull Runnable runnable, long uptimeMs) {
        TaskImpl task = new TaskImpl(runnable);
        mHandler.postDelayed(task, uptimeMs);
        Object token = new Object();
        TaskImpl task = new TaskImpl(mHandler, token);
        mHandler.postDelayed(runnable, token, uptimeMs);
        return task;
    }

    private void threadFunc() {
        Looper.prepare();
        synchronized (this) {
            mHandler = new Handler(Looper.myLooper());
            notifyAll();
        }
        Looper.loop();
    void quit() {
        mHandlerThread.quitSafely();
    }

    private static class TaskImpl implements Task, Runnable {
        private AtomicReference<Runnable> mRunnable = new AtomicReference<>();
    private static class TaskImpl implements Task {
        private final Handler mHandler;
        private final Object mToken;

        TaskImpl(@NonNull Runnable runnable) {
            mRunnable.set(runnable);
        public TaskImpl(Handler handler, Object token) {
            mHandler = handler;
            mToken = token;
        }

        @Override
        public void cancel() {
            mRunnable.set(null);
        }

        @Override
        public void run() {
            Runnable runnable = mRunnable.get();
            if (runnable != null) {
                runnable.run();
            }
        }
            mHandler.removeCallbacksAndMessages(mToken);
        }
    };
}