Merge "Fix ANR caused by VibrationThread"

This commit is contained in:
TreeHugger Robot
2021-01-21 07:08:26 +00:00
committed by Android (Google) Code Review
4 changed files with 157 additions and 65 deletions

View File

@@ -129,8 +129,10 @@ public class VibratorService extends IVibratorService.Stub {
Slog.d(TAG, "Vibration thread finished with status " + status);
}
synchronized (mLock) {
mThread = null;
reportFinishVibrationLocked(status);
if (mCurrentVibration != null && mCurrentVibration.id == vibrationId) {
mThread = null;
reportFinishVibrationLocked(status);
}
}
}
}

View File

@@ -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.");

View File

@@ -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.
// TODO(b/177807015): use exposed durations from IVibrator here instead
return 20 * effect.getPrimitiveEffects().size();
long duration = 0;
for (VibrationEffect.Composition.PrimitiveEffect primitive : primitives) {
// TODO(b/177807015): use exposed durations from IVibrator here instead
duration += 20 + primitive.delay;
}
return duration;
}
}

View File

@@ -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) {