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

Commit da188d45 authored by Lais Andrade's avatar Lais Andrade
Browse files

Fix ANR caused by VibrationThread 

The awaitUntil method that waits for vibration completion is waiting on
a CountDownLatch in a block synchrnized on the VibrationThread. This
causes any call to VibrationThread.cancel() to lock on this wait block
and triggers ANR when vibration is cancelled by the main thread.

The awaitUntil methods should only wait on the VibrationThread instance
so the calls to cancel() will notify and interrupt this wait.

Changed the VibrationThreadTest to test the cancel call on a longer
vibration pattern, that should cause the test to fail if the Thread is
not cancelled right away (i.e. similar conditions that would trigger an
ANR on the main thread).

Fix: 177974811
Test: VibrationThread
Change-Id: Id4346e60292e96df26f5c12e2e088bbf83f416c8
parent ee605b64

services/core/java/com/android/server/VibratorService.java

+4 −2
Original line number Diff line number Diff line
@@ -129,11 +129,13 @@ public class VibratorService extends IVibratorService.Stub { 
                Slog.d(TAG, "Vibration thread finished with status " + status);

            }

            synchronized (mLock) {

                if (mCurrentVibration != null && mCurrentVibration.id == vibrationId) {

                    mThread = null;

                    reportFinishVibrationLocked(status);

                }

            }

        }

    }



    /**

     * Implementation of {@link OnVibrationCompleteListener} with a weak reference to this service.

services/core/java/com/android/server/vibrator/VibrationThread.java

+34 −38
Original line number Diff line number Diff line
@@ -39,8 +39,6 @@ import com.google.android.collect.Lists; 
import java.util.ArrayList;

import java.util.List;

import java.util.PriorityQueue;

import java.util.concurrent.CountDownLatch;

import java.util.concurrent.TimeUnit;



/** Plays a {@link Vibration} in dedicated thread. */

// TODO(b/159207608): Make this package-private once vibrator services are moved to this package
@@ -76,7 +74,6 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
        void onVibrationEnded(long vibrationId, Vibration.Status status);

    }



    private final Object mLock = new Object();

    private final WorkSource mWorkSource = new WorkSource();

    private final PowerManager.WakeLock mWakeLock;

    private final IBatteryStats mBatteryStatsService;
@@ -84,7 +81,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
    private final VibrationCallbacks mCallbacks;

    private final SparseArray<VibratorController> mVibrators;



    @GuardedBy("mLock")

    @GuardedBy("this")

    @Nullable

    private VibrateStep mCurrentVibrateStep;

    @GuardedBy("this")
@@ -147,7 +144,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 


    /** Notify current vibration that a step has completed on given vibrator. */

    public void vibratorComplete(int vibratorId) {

        synchronized (mLock) {

        synchronized (this) {

            if (mCurrentVibrateStep != null) {

                mCurrentVibrateStep.vibratorComplete(vibratorId);

            }
@@ -171,7 +168,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
            final int stepCount = steps.size();

            for (int i = 0; i < stepCount; i++) {

                Step step = steps.get(i);

                synchronized (mLock) {

                synchronized (this) {

                    if (step instanceof VibrateStep) {

                        mCurrentVibrateStep = (VibrateStep) step;

                    } else {
@@ -315,27 +312,6 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
        }

    }



    /**

     * Sleeps until given {@link CountDownLatch} has finished or {@code wakeUpTime} was reached.

     *

     * <p>This stops immediately when {@link #cancel()} is called.

     */

    private void awaitUntil(CountDownLatch counter, long wakeUpTime) {

        synchronized (this) {

            long durationRemaining = wakeUpTime - SystemClock.uptimeMillis();

            while (counter.getCount() > 0 && durationRemaining > 0) {

                try {

                    counter.await(durationRemaining, TimeUnit.MILLISECONDS);

                } catch (InterruptedException e) {

                }

                if (mForceStop) {

                    break;

                }

                durationRemaining = wakeUpTime - SystemClock.uptimeMillis();

            }

        }

    }



    private void noteVibratorOn(long duration) {

        try {

            mBatteryStatsService.noteVibratorOn(mVibration.uid, duration);
@@ -371,12 +347,10 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
    private final class SingleVibrateStep implements VibrateStep {

        private final VibratorController mVibrator;

        private final VibrationEffect mEffect;

        private final CountDownLatch mCounter;



        SingleVibrateStep(VibratorController vibrator, VibrationEffect effect) {

            mVibrator = vibrator;

            mEffect = effect;

            mCounter = new CountDownLatch(1);

        }



        @Override
@@ -390,7 +364,9 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
                return;

            }

            mVibrator.off();

            mCounter.countDown();

            synchronized (VibrationThread.this) {

                VibrationThread.this.notify();

            }

        }



        @Override
@@ -408,7 +384,11 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
                    noteVibratorOn(duration);

                    // Vibration is playing with no need to control amplitudes, just wait for native

                    // callback or timeout.

                    awaitUntil(mCounter, startTime + duration + CALLBACKS_EXTRA_TIMEOUT);

                    waitUntil(startTime + duration + CALLBACKS_EXTRA_TIMEOUT);

                    if (mForceStop) {

                        mVibrator.off();

                        return Vibration.Status.CANCELLED;

                    }

                    return Vibration.Status.FINISHED;

                }
@@ -499,14 +479,15 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
    /** Represent a synchronized vibration step on multiple vibrators. */

    private final class SyncedVibrateStep implements VibrateStep {

        private final SparseArray<VibrationEffect> mEffects;

        private final CountDownLatch mActiveVibratorCounter;



        private final int mRequiredCapabilities;

        private final int[] mVibratorIds;



        @GuardedBy("VibrationThread.this")

        private int mActiveVibratorCounter;



        SyncedVibrateStep(SparseArray<VibrationEffect> effects) {

            mEffects = effects;

            mActiveVibratorCounter = new CountDownLatch(mEffects.size());

            mActiveVibratorCounter = mEffects.size();

            // TODO(b/159207608): Calculate required capabilities for syncing this step.

            mRequiredCapabilities = 0;

            mVibratorIds = new int[effects.size()];
@@ -527,7 +508,11 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
                return;

            }

            mVibrators.get(vibratorId).off();

            mActiveVibratorCounter.countDown();

            synchronized (VibrationThread.this) {

                if (--mActiveVibratorCounter <= 0) {

                    VibrationThread.this.notify();

                }

            }

        }



        @Override
@@ -556,7 +541,9 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
                    AmplitudeStep nextStep = step.nextStep();

                    if (nextStep == null) {

                        // This vibrator has finished playing the effect for this step.

                        mActiveVibratorCounter.countDown();

                        synchronized (VibrationThread.this) {

                            mActiveVibratorCounter--;

                        }

                    } else {

                        nextSteps.add(nextStep);

                    }
@@ -564,7 +551,16 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 


                // All OneShot and Waveform effects have finished. Just wait for the other effects

                // to end via native callbacks before finishing this synced step.

                awaitUntil(mActiveVibratorCounter, startTime + timeout + CALLBACKS_EXTRA_TIMEOUT);

                synchronized (VibrationThread.this) {

                    if (mActiveVibratorCounter > 0) {

                        waitUntil(startTime + timeout + CALLBACKS_EXTRA_TIMEOUT);

                    }

                }

                if (mForceStop) {

                    stopAllVibrators();

                    return Vibration.Status.CANCELLED;

                }



                return Vibration.Status.FINISHED;

            } finally {

                if (timeout > 0) {
@@ -779,7 +775,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi 
                    Slog.d(TAG, "DelayStep of " + mDelay + "ms starting...");

                }

                waitUntil(SystemClock.uptimeMillis() + mDelay);

                return Vibration.Status.FINISHED;

                return mForceStop ? Vibration.Status.CANCELLED : Vibration.Status.FINISHED;

            } finally {

                if (DEBUG) {

                    Slog.d(TAG, "DelayStep done.");

services/core/java/com/android/server/vibrator/VibratorController.java

+10 −4
Original line number Diff line number Diff line
@@ -272,12 +272,18 @@ public final class VibratorController { 
            return 0;

        }

        synchronized (mLock) {

            mNativeWrapper.compose(effect.getPrimitiveEffects().toArray(

                    new VibrationEffect.Composition.PrimitiveEffect[0]), vibrationId);

            VibrationEffect.Composition.PrimitiveEffect[] primitives =

                    effect.getPrimitiveEffects().toArray(

                            new VibrationEffect.Composition.PrimitiveEffect[0]);

            mNativeWrapper.compose(primitives, vibrationId);

            notifyVibratorOnLocked();

            // Compose don't actually give us an estimated duration, so we just guess here.

            long duration = 0;

            for (VibrationEffect.Composition.PrimitiveEffect primitive : primitives) {

                // TODO(b/177807015): use exposed durations from IVibrator here instead

            return 20 * effect.getPrimitiveEffects().size();

                duration += 20 + primitive.delay;

            }

            return duration;

        }

    }

services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java

+109 −21
Original line number Diff line number Diff line
@@ -212,6 +212,60 @@ public class VibrationThreadTest { 
        }

    }



    @Test

    public void vibrate_singleVibratorPredefinedCancel_cancelsVibrationImmediately()

            throws Exception {

        mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS);



        long vibrationId = 1;

        VibrationEffect effect = VibrationEffect.startComposition()

                .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100)

                .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100)

                .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100)

                .compose();

        VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect);



        Thread.sleep(20);

        assertTrue(vibrationThread.isAlive());

        assertTrue(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating());



        // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should

        // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately.

        Thread cancellingThread = new Thread(() -> vibrationThread.cancel());

        cancellingThread.start();



        waitForCompletion(vibrationThread, 20);

        waitForCompletion(cancellingThread);



        verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));

        assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating());

    }



    @Test

    public void vibrate_singleVibratorWaveformCancel_cancelsVibrationImmediately()

            throws Exception {

        mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS);



        long vibrationId = 1;

        VibrationEffect effect = VibrationEffect.createWaveform(new long[]{100}, new int[]{100}, 0);

        VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect);



        Thread.sleep(20);

        assertTrue(vibrationThread.isAlive());

        assertTrue(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating());



        // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should

        // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately.

        Thread cancellingThread = new Thread(() -> vibrationThread.cancel());

        cancellingThread.start();



        waitForCompletion(vibrationThread, 20);

        waitForCompletion(cancellingThread);



        verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));

        assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating());

    }



    @Test

    public void vibrate_singleVibratorPrebaked_runsVibration() throws Exception {

        mVibratorProviders.get(1).setSupportedEffects(VibrationEffect.EFFECT_THUD);
@@ -544,36 +598,70 @@ public class VibrationThreadTest { 
    }



    @Test

    public void vibrate_multipleCancelled_allVibratorsStopped() throws Exception {

        mockVibrators(1, 2, 3);

    public void vibrate_multiplePredefinedCancel_cancelsVibrationImmediately() throws Exception {

        mockVibrators(1, 2);

        mVibratorProviders.get(1).setSupportedEffects(VibrationEffect.EFFECT_CLICK);

        mVibratorProviders.get(2).setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS);



        long vibrationId = 1;

        CombinedVibrationEffect effect = CombinedVibrationEffect.startSynced()

                .addVibrator(1, VibrationEffect.get(VibrationEffect.EFFECT_CLICK))

                .addVibrator(2, VibrationEffect.startComposition()

                        .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100)

                        .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100)

                        .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100)

                        .compose())

                .combine();

        VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect);



        Thread.sleep(10);

        assertTrue(vibrationThread.isAlive());

        assertTrue(vibrationThread.getVibrators().get(1).isVibrating());

        assertTrue(vibrationThread.getVibrators().get(2).isVibrating());



        // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should

        // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately.

        Thread cancellingThread = new Thread(() -> vibrationThread.cancel());

        cancellingThread.start();



        waitForCompletion(vibrationThread, 20);

        waitForCompletion(cancellingThread);



        verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));

        assertFalse(vibrationThread.getVibrators().get(1).isVibrating());

        assertFalse(vibrationThread.getVibrators().get(2).isVibrating());

    }



    @Test

    public void vibrate_multipleWaveformCancel_cancelsVibrationImmediately() throws Exception {

        mockVibrators(1, 2);

        mVibratorProviders.get(1).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);

        mVibratorProviders.get(2).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);

        mVibratorProviders.get(3).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);



        long vibrationId = 1;

        CombinedVibrationEffect effect = CombinedVibrationEffect.startSynced()

                .addVibrator(1, VibrationEffect.createWaveform(

                        new long[]{5, 10}, new int[]{1, 2}, 0))

                .addVibrator(2, VibrationEffect.createWaveform(

                        new long[]{20, 30}, new int[]{3, 4}, 0))

                .addVibrator(3, VibrationEffect.createWaveform(

                        new long[]{10, 40}, new int[]{5, 6}, 0))

                        new long[]{100, 100}, new int[]{1, 2}, 0))

                .addVibrator(2, VibrationEffect.createOneShot(100, 100))

                .combine();

        VibrationThread thread = startThreadAndDispatcher(vibrationId, effect);

        VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect);



        Thread.sleep(15);

        assertTrue(thread.isAlive());

        assertTrue(thread.getVibrators().get(1).isVibrating());

        assertTrue(thread.getVibrators().get(2).isVibrating());

        assertTrue(thread.getVibrators().get(3).isVibrating());

        Thread.sleep(10);

        assertTrue(vibrationThread.isAlive());

        assertTrue(vibrationThread.getVibrators().get(1).isVibrating());

        assertTrue(vibrationThread.getVibrators().get(2).isVibrating());



        thread.cancel();

        waitForCompletion(thread);

        assertFalse(thread.getVibrators().get(1).isVibrating());

        assertFalse(thread.getVibrators().get(2).isVibrating());

        assertFalse(thread.getVibrators().get(3).isVibrating());

        // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should

        // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately.

        Thread cancellingThread = new Thread(() -> vibrationThread.cancel());

        cancellingThread.start();



        waitForCompletion(vibrationThread, 20);

        waitForCompletion(cancellingThread);



        verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED));

        assertFalse(vibrationThread.getVibrators().get(1).isVibrating());

        assertFalse(vibrationThread.getVibrators().get(2).isVibrating());

    }



    @Test
@@ -621,11 +709,11 @@ public class VibrationThreadTest { 
        return thread;

    }



    private void waitForCompletion(VibrationThread thread) {

    private void waitForCompletion(Thread thread) {

        waitForCompletion(thread, TEST_TIMEOUT_MILLIS);

    }



    private void waitForCompletion(VibrationThread thread, long timeout) {

    private void waitForCompletion(Thread thread, long timeout) {

        try {

            thread.join(timeout);

        } catch (InterruptedException e) {