Merge "Fix session start race condition" into main
diff --git a/services/core/java/com/android/server/vibrator/VendorVibrationSession.java b/services/core/java/com/android/server/vibrator/VendorVibrationSession.java
index bda3d44..621a128 100644
--- a/services/core/java/com/android/server/vibrator/VendorVibrationSession.java
+++ b/services/core/java/com/android/server/vibrator/VendorVibrationSession.java
@@ -51,6 +51,7 @@
final class VendorVibrationSession extends IVibrationSession.Stub
implements VibrationSession, CancellationSignal.OnCancelListener, IBinder.DeathRecipient {
private static final String TAG = "VendorVibrationSession";
+ private static final boolean DEBUG = false;
/** Calls into VibratorManager functionality needed for playing an {@link ExternalVibration}. */
interface VibratorManagerHooks {
@@ -73,8 +74,8 @@
private final ICancellationSignal mCancellationSignal = CancellationSignal.createTransport();
private final int[] mVibratorIds;
private final long mCreateUptime;
- private final long mCreateTime; // for debugging
- private final IVibrationSessionCallback mCallback;
+ private final long mCreateTime;
+ private final VendorCallbackWrapper mCallback;
private final CallerInfo mCallerInfo;
private final VibratorManagerHooks mManagerHooks;
private final DeviceAdapter mDeviceAdapter;
@@ -88,11 +89,11 @@
@GuardedBy("mLock")
private boolean mEndedByVendor;
@GuardedBy("mLock")
- private long mStartTime; // for debugging
+ private long mStartTime;
@GuardedBy("mLock")
private long mEndUptime;
@GuardedBy("mLock")
- private long mEndTime; // for debugging
+ private long mEndTime;
@GuardedBy("mLock")
private VibrationStepConductor mConductor;
@@ -103,7 +104,7 @@
mCreateTime = System.currentTimeMillis();
mVibratorIds = deviceAdapter.getAvailableVibratorIds();
mHandler = handler;
- mCallback = callback;
+ mCallback = new VendorCallbackWrapper(callback, handler);
mCallerInfo = callerInfo;
mManagerHooks = managerHooks;
mDeviceAdapter = deviceAdapter;
@@ -119,7 +120,9 @@
@Override
public void finishSession() {
- Slog.d(TAG, "Session finish requested, ending vibration session...");
+ if (DEBUG) {
+ Slog.d(TAG, "Session finish requested, ending vibration session...");
+ }
// Do not abort session in HAL, wait for ongoing vibration requests to complete.
// This might take a while to end the session, but it can be aborted by cancelSession.
requestEndSession(Status.FINISHED, /* shouldAbort= */ false, /* isVendorRequest= */ true);
@@ -127,7 +130,9 @@
@Override
public void cancelSession() {
- Slog.d(TAG, "Session cancel requested, aborting vibration session...");
+ if (DEBUG) {
+ Slog.d(TAG, "Session cancel requested, aborting vibration session...");
+ }
// Always abort session in HAL while cancelling it.
// This might be triggered after finishSession was already called.
requestEndSession(Status.CANCELLED_BY_USER, /* shouldAbort= */ true,
@@ -156,7 +161,7 @@
@Override
public IBinder getCallerToken() {
- return mCallback.asBinder();
+ return mCallback.getBinderToken();
}
@Override
@@ -176,36 +181,30 @@
@Override
public void onCancel() {
- Slog.d(TAG, "Session cancellation signal received, aborting vibration session...");
+ if (DEBUG) {
+ Slog.d(TAG, "Session cancellation signal received, aborting vibration session...");
+ }
requestEndSession(Status.CANCELLED_BY_USER, /* shouldAbort= */ true,
/* isVendorRequest= */ true);
}
@Override
public void binderDied() {
- Slog.d(TAG, "Session binder died, aborting vibration session...");
+ if (DEBUG) {
+ Slog.d(TAG, "Session binder died, aborting vibration session...");
+ }
requestEndSession(Status.CANCELLED_BINDER_DIED, /* shouldAbort= */ true,
/* isVendorRequest= */ false);
}
@Override
public boolean linkToDeath() {
- try {
- mCallback.asBinder().linkToDeath(this, 0);
- } catch (RemoteException e) {
- Slog.e(TAG, "Error linking session to token death", e);
- return false;
- }
- return true;
+ return mCallback.linkToDeath(this);
}
@Override
public void unlinkToDeath() {
- try {
- mCallback.asBinder().unlinkToDeath(this, 0);
- } catch (NoSuchElementException e) {
- Slog.wtf(TAG, "Failed to unlink session to token death", e);
- }
+ mCallback.unlinkToDeath(this);
}
@Override
@@ -219,26 +218,37 @@
@Override
public void notifyVibratorCallback(int vibratorId, long vibrationId, long stepId) {
- Slog.d(TAG, "Vibration callback received for vibration " + vibrationId + " step " + stepId
- + " on vibrator " + vibratorId + ", ignoring...");
+ if (DEBUG) {
+ Slog.d(TAG, "Vibration callback received for vibration " + vibrationId
+ + " step " + stepId + " on vibrator " + vibratorId + ", ignoring...");
+ }
}
@Override
public void notifySyncedVibratorsCallback(long vibrationId) {
- Slog.d(TAG, "Synced vibration callback received for vibration " + vibrationId
- + ", ignoring...");
+ if (DEBUG) {
+ Slog.d(TAG, "Synced vibration callback received for vibration " + vibrationId
+ + ", ignoring...");
+ }
}
@Override
public void notifySessionCallback() {
- Slog.d(TAG, "Session callback received, ending vibration session...");
+ if (DEBUG) {
+ Slog.d(TAG, "Session callback received, ending vibration session...");
+ }
synchronized (mLock) {
// If end was not requested then the HAL has cancelled the session.
- maybeSetEndRequestLocked(Status.CANCELLED_BY_UNKNOWN_REASON,
+ notifyEndRequestLocked(Status.CANCELLED_BY_UNKNOWN_REASON,
/* isVendorRequest= */ false);
maybeSetStatusToRequestedLocked();
clearVibrationConductor();
- mHandler.post(() -> mManagerHooks.onSessionReleased(mSessionId));
+ final Status endStatus = mStatus;
+ mHandler.post(() -> {
+ mManagerHooks.onSessionReleased(mSessionId);
+ // Only trigger client callback after session is released in the manager.
+ mCallback.notifyFinished(endStatus);
+ });
}
}
@@ -271,7 +281,7 @@
public boolean isEnded() {
synchronized (mLock) {
- return mStatus != Status.RUNNING;
+ return mEndTime > 0;
}
}
@@ -297,19 +307,17 @@
// Session already ended, skip start callbacks.
isAlreadyEnded = true;
} else {
+ if (DEBUG) {
+ Slog.d(TAG, "Session started at the HAL");
+ }
mStartTime = System.currentTimeMillis();
- // Run client callback in separate thread.
- mHandler.post(() -> {
- try {
- mCallback.onStarted(this);
- } catch (RemoteException e) {
- Slog.e(TAG, "Error notifying vendor session started", e);
- }
- });
+ mCallback.notifyStarted(this);
}
}
if (isAlreadyEnded) {
- Slog.d(TAG, "Session already ended after starting the HAL, aborting...");
+ if (DEBUG) {
+ Slog.d(TAG, "Session already ended after starting the HAL, aborting...");
+ }
mHandler.post(() -> mManagerHooks.endSession(mSessionId, /* shouldAbort= */ true));
}
}
@@ -337,8 +345,10 @@
public boolean maybeSetVibrationConductor(VibrationStepConductor conductor) {
synchronized (mLock) {
if (mConductor != null) {
- Slog.d(TAG, "Session still dispatching previous vibration, new vibration "
- + conductor.getVibration().id + " ignored");
+ if (DEBUG) {
+ Slog.d(TAG, "Session still dispatching previous vibration, new vibration "
+ + conductor.getVibration().id + " ignored");
+ }
return false;
}
mConductor = conductor;
@@ -347,53 +357,45 @@
}
private void requestEndSession(Status status, boolean shouldAbort, boolean isVendorRequest) {
- Slog.d(TAG, "Session end request received with status " + status);
- boolean shouldTriggerSessionHook = false;
+ if (DEBUG) {
+ Slog.d(TAG, "Session end request received with status " + status);
+ }
synchronized (mLock) {
- maybeSetEndRequestLocked(status, isVendorRequest);
+ notifyEndRequestLocked(status, isVendorRequest);
if (!isEnded() && isStarted()) {
// Trigger session hook even if it was already triggered, in case a second request
// is aborting the ongoing/ending session. This might cause it to end right away.
// Wait for HAL callback before setting the end status.
- shouldTriggerSessionHook = true;
+ if (DEBUG) {
+ Slog.d(TAG, "Requesting HAL session end with abort=" + shouldAbort);
+ }
+ mHandler.post(() -> mManagerHooks.endSession(mSessionId, shouldAbort));
} else {
- // Session not active in the HAL, set end status right away.
+ // Session not active in the HAL, try to set end status right away.
maybeSetStatusToRequestedLocked();
+ // Use status used to end this session, which might be different from requested.
+ mCallback.notifyFinished(mStatus);
}
}
- if (shouldTriggerSessionHook) {
- Slog.d(TAG, "Requesting HAL session end with abort=" + shouldAbort);
- mHandler.post(() -> mManagerHooks.endSession(mSessionId, shouldAbort));
- }
}
@GuardedBy("mLock")
- private void maybeSetEndRequestLocked(Status status, boolean isVendorRequest) {
+ private void notifyEndRequestLocked(Status status, boolean isVendorRequest) {
if (mEndStatusRequest != null) {
- // End already requested, keep first requested status and time.
+ // End already requested, keep first requested status.
return;
}
- Slog.d(TAG, "Session end request accepted for status " + status);
+ if (DEBUG) {
+ Slog.d(TAG, "Session end request accepted for status " + status);
+ }
mEndStatusRequest = status;
mEndedByVendor = isVendorRequest;
- mEndTime = System.currentTimeMillis();
- mEndUptime = SystemClock.uptimeMillis();
+ mCallback.notifyFinishing();
if (mConductor != null) {
// Vibration is being dispatched when session end was requested, cancel it.
mConductor.notifyCancelled(new Vibration.EndInfo(status),
/* immediate= */ status != Status.FINISHED);
}
- if (isStarted()) {
- // Only trigger "finishing" callback if session started.
- // Run client callback in separate thread.
- mHandler.post(() -> {
- try {
- mCallback.onFinishing();
- } catch (RemoteException e) {
- Slog.e(TAG, "Error notifying vendor session is finishing", e);
- }
- });
- }
}
@GuardedBy("mLock")
@@ -406,40 +408,123 @@
// No end status was requested, nothing to set.
return;
}
- Slog.d(TAG, "Session end request applied for status " + mEndStatusRequest);
+ if (DEBUG) {
+ Slog.d(TAG, "Session end request applied for status " + mEndStatusRequest);
+ }
mStatus = mEndStatusRequest;
- // Run client callback in separate thread.
- final Status endStatus = mStatus;
- mHandler.post(() -> {
- try {
- mCallback.onFinished(toSessionStatus(endStatus));
- } catch (RemoteException e) {
- Slog.e(TAG, "Error notifying vendor session finished", e);
- }
- });
+ mEndTime = System.currentTimeMillis();
+ mEndUptime = SystemClock.uptimeMillis();
}
- @android.os.vibrator.VendorVibrationSession.Status
- private static int toSessionStatus(Status status) {
- // Exhaustive switch to cover all possible internal status.
- return switch (status) {
- case FINISHED
- -> android.os.vibrator.VendorVibrationSession.STATUS_SUCCESS;
- case IGNORED_UNSUPPORTED
- -> STATUS_UNSUPPORTED;
- case CANCELLED_BINDER_DIED, CANCELLED_BY_APP_OPS, CANCELLED_BY_USER,
- CANCELLED_SUPERSEDED, CANCELLED_BY_FOREGROUND_USER, CANCELLED_BY_SCREEN_OFF,
- CANCELLED_BY_SETTINGS_UPDATE, CANCELLED_BY_UNKNOWN_REASON
- -> android.os.vibrator.VendorVibrationSession.STATUS_CANCELED;
- case IGNORED_APP_OPS, IGNORED_BACKGROUND, IGNORED_FOR_EXTERNAL, IGNORED_FOR_ONGOING,
- IGNORED_FOR_POWER, IGNORED_FOR_SETTINGS, IGNORED_FOR_HIGHER_IMPORTANCE,
- IGNORED_FOR_RINGER_MODE, IGNORED_FROM_VIRTUAL_DEVICE, IGNORED_SUPERSEDED,
- IGNORED_MISSING_PERMISSION, IGNORED_ON_WIRELESS_CHARGER
- -> android.os.vibrator.VendorVibrationSession.STATUS_IGNORED;
- case UNKNOWN, IGNORED_ERROR_APP_OPS, IGNORED_ERROR_CANCELLING, IGNORED_ERROR_SCHEDULING,
- IGNORED_ERROR_TOKEN, FORWARDED_TO_INPUT_DEVICES, FINISHED_UNEXPECTED, RUNNING
- -> android.os.vibrator.VendorVibrationSession.STATUS_UNKNOWN_ERROR;
- };
+ /**
+ * Wrapper class to handle client callbacks asynchronously.
+ *
+ * <p>This class is also responsible for link/unlink to the client process binder death, and for
+ * making sure the callbacks are only triggered once. The conversion between session status and
+ * the API status code is also defined here.
+ */
+ private static final class VendorCallbackWrapper {
+ private final IVibrationSessionCallback mCallback;
+ private final Handler mHandler;
+
+ private boolean mIsStarted;
+ private boolean mIsFinishing;
+ private boolean mIsFinished;
+
+ VendorCallbackWrapper(@NonNull IVibrationSessionCallback callback,
+ @NonNull Handler handler) {
+ mCallback = callback;
+ mHandler = handler;
+ }
+
+ synchronized IBinder getBinderToken() {
+ return mCallback.asBinder();
+ }
+
+ synchronized boolean linkToDeath(DeathRecipient recipient) {
+ try {
+ mCallback.asBinder().linkToDeath(recipient, 0);
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Error linking session to token death", e);
+ return false;
+ }
+ return true;
+ }
+
+ synchronized void unlinkToDeath(DeathRecipient recipient) {
+ try {
+ mCallback.asBinder().unlinkToDeath(recipient, 0);
+ } catch (NoSuchElementException e) {
+ Slog.wtf(TAG, "Failed to unlink session to token death", e);
+ }
+ }
+
+ synchronized void notifyStarted(IVibrationSession session) {
+ if (mIsStarted) {
+ return;
+ }
+ mIsStarted = true;
+ mHandler.post(() -> {
+ try {
+ mCallback.onStarted(session);
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Error notifying vendor session started", e);
+ }
+ });
+ }
+
+ synchronized void notifyFinishing() {
+ if (!mIsStarted || mIsFinishing || mIsFinished) {
+ // Ignore if never started or if already finishing or finished.
+ return;
+ }
+ mIsFinishing = true;
+ mHandler.post(() -> {
+ try {
+ mCallback.onFinishing();
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Error notifying vendor session is finishing", e);
+ }
+ });
+ }
+
+ synchronized void notifyFinished(Status status) {
+ if (mIsFinished) {
+ return;
+ }
+ mIsFinished = true;
+ mHandler.post(() -> {
+ try {
+ mCallback.onFinished(toSessionStatus(status));
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Error notifying vendor session finished", e);
+ }
+ });
+ }
+
+ @android.os.vibrator.VendorVibrationSession.Status
+ private static int toSessionStatus(Status status) {
+ // Exhaustive switch to cover all possible internal status.
+ return switch (status) {
+ case FINISHED
+ -> android.os.vibrator.VendorVibrationSession.STATUS_SUCCESS;
+ case IGNORED_UNSUPPORTED
+ -> STATUS_UNSUPPORTED;
+ case CANCELLED_BINDER_DIED, CANCELLED_BY_APP_OPS, CANCELLED_BY_USER,
+ CANCELLED_SUPERSEDED, CANCELLED_BY_FOREGROUND_USER, CANCELLED_BY_SCREEN_OFF,
+ CANCELLED_BY_SETTINGS_UPDATE, CANCELLED_BY_UNKNOWN_REASON
+ -> android.os.vibrator.VendorVibrationSession.STATUS_CANCELED;
+ case IGNORED_APP_OPS, IGNORED_BACKGROUND, IGNORED_FOR_EXTERNAL, IGNORED_FOR_ONGOING,
+ IGNORED_FOR_POWER, IGNORED_FOR_SETTINGS, IGNORED_FOR_HIGHER_IMPORTANCE,
+ IGNORED_FOR_RINGER_MODE, IGNORED_FROM_VIRTUAL_DEVICE, IGNORED_SUPERSEDED,
+ IGNORED_MISSING_PERMISSION, IGNORED_ON_WIRELESS_CHARGER
+ -> android.os.vibrator.VendorVibrationSession.STATUS_IGNORED;
+ case UNKNOWN, IGNORED_ERROR_APP_OPS, IGNORED_ERROR_CANCELLING,
+ IGNORED_ERROR_SCHEDULING, IGNORED_ERROR_TOKEN, FORWARDED_TO_INPUT_DEVICES,
+ FINISHED_UNEXPECTED, RUNNING
+ -> android.os.vibrator.VendorVibrationSession.STATUS_UNKNOWN_ERROR;
+ };
+ }
}
/**
@@ -499,7 +584,7 @@
@Override
public void logMetrics(VibratorFrameworkStatsLogger statsLogger) {
if (mStartTime > 0) {
- // Only log sessions that have started.
+ // Only log sessions that have started in the HAL.
statsLogger.logVibrationVendorSessionStarted(mCallerInfo.uid);
statsLogger.logVibrationVendorSessionVibrations(mCallerInfo.uid,
mVibrations.size());