Handle binder death with the conductor rather than VibrationThread.

The conductor is directly associated with the vibration+binder token,
unlike the thread, so the signal is guaranteed to be correctly attributed.

Bug: 193792066
Test: atest, presubmit
Change-Id: I34bdf2c942e1c8e0db9526c9e84e92f69718f4b4
This commit is contained in:
Simon Bowden
2022-03-01 15:37:42 +00:00
parent fad808ecd8
commit 8e3d1870bd
3 changed files with 24 additions and 27 deletions

View File

@@ -20,6 +20,7 @@ import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.Build;
import android.os.CombinedVibration;
import android.os.IBinder;
import android.os.VibrationEffect;
import android.os.vibrator.PrebakedSegment;
import android.os.vibrator.PrimitiveSegment;
@@ -46,7 +47,7 @@ import java.util.Queue;
* VibrationThread. The only thread-safe methods for calling from other threads are the "notify"
* methods (which should never be used from the VibrationThread thread).
*/
final class VibrationStepConductor {
final class VibrationStepConductor implements IBinder.DeathRecipient {
private static final boolean DEBUG = VibrationThread.DEBUG;
private static final String TAG = VibrationThread.TAG;
@@ -289,6 +290,19 @@ final class VibrationStepConductor {
}
}
/**
* Binder death notification. VibrationThread registers this when it's running a conductor.
* Note that cancellation could theoretically happen immediately, before the conductor has
* started, but in this case it will be processed in the first signals loop.
*/
@Override
public void binderDied() {
if (DEBUG) {
Slog.d(TAG, "Binder died, cancelling vibration...");
}
notifyCancelled(/* immediate= */ false);
}
/**
* Notify the execution that cancellation is requested. This will be acted upon
* asynchronously in the VibrationThread.

View File

@@ -33,7 +33,7 @@ import java.util.NoSuchElementException;
import java.util.Objects;
/** Plays a {@link Vibration} in dedicated thread. */
final class VibrationThread extends Thread implements IBinder.DeathRecipient {
final class VibrationThread extends Thread {
static final String TAG = "VibrationThread";
static final boolean DEBUG = false;
@@ -118,23 +118,6 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
mVibratorManagerHooks = vibratorManagerHooks;
}
@Override
public void binderDied() {
if (DEBUG) {
Slog.d(TAG, "Binder died, cancelling vibration...");
}
// The binder death link only exists while the conductor is set.
// TODO: move the death linking to be associated with the conductor directly, this
// awkwardness will go away.
VibrationStepConductor conductor;
synchronized (mLock) {
conductor = mRequestedActiveConductor;
}
if (conductor != null) {
conductor.notifyCancelled(/* immediate= */ false);
}
}
/**
* Sets/activates the current vibration. Must only be called after receiving
* onVibratorsReleased from the previous vibration.
@@ -268,7 +251,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
private void runCurrentVibrationWithWakeLockAndDeathLink() {
IBinder vibrationBinderToken = mExecutingConductor.getVibration().token;
try {
vibrationBinderToken.linkToDeath(this, 0);
vibrationBinderToken.linkToDeath(mExecutingConductor, 0);
} catch (RemoteException e) {
Slog.e(TAG, "Error linking vibration to token death", e);
clientVibrationCompleteIfNotAlready(Vibration.Status.IGNORED_ERROR_TOKEN);
@@ -280,7 +263,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
playVibration();
} finally {
try {
vibrationBinderToken.unlinkToDeath(this, 0);
vibrationBinderToken.unlinkToDeath(mExecutingConductor, 0);
} catch (NoSuchElementException e) {
Slog.wtf(TAG, "Failed to unlink token", e);
}

View File

@@ -636,14 +636,14 @@ public class VibrationThreadTest {
long vibrationId = 1;
VibrationEffect effect = VibrationEffect.createWaveform(new long[]{5}, new int[]{100}, 0);
startThreadAndDispatcher(vibrationId, effect);
VibrationStepConductor conductor = startThreadAndDispatcher(vibrationId, effect);
assertTrue(waitUntil(() -> fakeVibrator.getAmplitudes().size() > 2, TEST_TIMEOUT_MILLIS));
// Vibration still running after 2 cycles.
assertTrue(mThread.isRunningVibrationId(vibrationId));
assertTrue(mControllers.get(VIBRATOR_ID).isVibrating());
mThread.binderDied();
conductor.binderDied();
waitForCompletion();
assertFalse(mControllers.get(VIBRATOR_ID).isVibrating());
@@ -1128,17 +1128,17 @@ public class VibrationThreadTest {
public void vibrate_binderDied_cancelsVibration() throws Exception {
long vibrationId = 1;
VibrationEffect effect = VibrationEffect.createWaveform(new long[]{5}, new int[]{100}, 0);
startThreadAndDispatcher(vibrationId, effect);
VibrationStepConductor conductor = startThreadAndDispatcher(vibrationId, effect);
assertTrue(waitUntil(() -> mControllers.get(VIBRATOR_ID).isVibrating(),
TEST_TIMEOUT_MILLIS));
assertTrue(mThread.isRunningVibrationId(vibrationId));
mThread.binderDied();
conductor.binderDied();
waitForCompletion();
verify(mVibrationToken).linkToDeath(same(mThread), eq(0));
verify(mVibrationToken).unlinkToDeath(same(mThread), eq(0));
verify(mVibrationToken).linkToDeath(same(conductor), eq(0));
verify(mVibrationToken).unlinkToDeath(same(conductor), eq(0));
verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED);
assertFalse(mVibratorProviders.get(VIBRATOR_ID).getEffectSegments(vibrationId).isEmpty());
assertFalse(mControllers.get(VIBRATOR_ID).isVibrating());