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

Commit 3b8711b3 authored by Lais Andrade's avatar Lais Andrade
Browse files

Move interaction with vibrator HAL outside VibratorController lock 

The vibrator HAL is getting stuck to a call to IVibrator.on, which is
done inside the VibratorController.mLock. This causes any read call,
including toString, to be blocked on the vibrator HAL.

Also introduding more debug logs to the service and thread.

Bug: 205323128
Test: manual
Change-Id: I2783bc17782c19defe04d4298a165d84751ddde9
parent 86ef373a

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

+3 −0
Original line number Diff line number Diff line
@@ -287,6 +287,9 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { 
                }

                // If we waited, the queue may have changed, so let the loop run again.

                if (waitTime <= 0) {

                    if (DEBUG) {

                        Slog.d(TAG, "Play vibration consuming next step...");

                    }

                    mStepQueue.consumeNext();

                }

                Vibration.Status status = mStop ? Vibration.Status.CANCELLED

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

+74 −99
Original line number Diff line number Diff line
@@ -40,21 +40,22 @@ final class VibratorController { 
    private static final int SUGGESTED_FREQUENCY_SAFE_RANGE = 200;



    private final Object mLock = new Object();

    private final NativeWrapper mNativeWrapper;



    @GuardedBy("mLock")

    private VibratorInfo mVibratorInfo;

    @GuardedBy("mLock")

    private boolean mVibratorInfoLoadSuccessful;

    @GuardedBy("mLock")

    private final NativeWrapper mNativeWrapper;



    // Vibrator state listeners that support concurrent updates and broadcasts, but should lock

    // while broadcasting to guarantee delivery order.

    private final RemoteCallbackList<IVibratorStateListener> mVibratorStateListeners =

            new RemoteCallbackList<>();

    @GuardedBy("mLock")

    private boolean mIsVibrating;

    @GuardedBy("mLock")

    private boolean mIsUnderExternalControl;

    @GuardedBy("mLock")

    private float mCurrentAmplitude;



    // Vibrator state variables that are updated from synchronized blocks but can be read anytime

    // for a snippet of the current known vibrator state/info.

    private volatile VibratorInfo mVibratorInfo;

    private volatile boolean mVibratorInfoLoadSuccessful;

    private volatile boolean mIsVibrating;

    private volatile boolean mIsUnderExternalControl;

    private volatile float mCurrentAmplitude;



    /** Listener for vibration completion callbacks from native. */

    public interface OnVibrationCompleteListener {
@@ -86,24 +87,25 @@ final class VibratorController { 


    /** Register state listener for this vibrator. */

    public boolean registerVibratorStateListener(IVibratorStateListener listener) {

        synchronized (mLock) {

        final long token = Binder.clearCallingIdentity();

        try {

            // Register the listener and send the first state atomically, to avoid potentially

            // out of order broadcasts in between.

            synchronized (mLock) {

                if (!mVibratorStateListeners.register(listener)) {

                    return false;

                }

                // Notify its callback after new client registered.

                notifyStateListenerLocked(listener);

                notifyStateListener(listener, mIsVibrating);

            }

            return true;

        } finally {

            Binder.restoreCallingIdentity(token);

        }

    }

    }



    /** Remove registered state listener for this vibrator. */

    public boolean unregisterVibratorStateListener(IVibratorStateListener listener) {

        synchronized (mLock) {

        final long token = Binder.clearCallingIdentity();

        try {

            return mVibratorStateListeners.unregister(listener);
@@ -111,10 +113,13 @@ final class VibratorController { 
            Binder.restoreCallingIdentity(token);

        }

    }

    }



    /** Reruns the query to the vibrator to load the {@link VibratorInfo}, if not yet successful. */

    public void reloadVibratorInfoIfNeeded() {

        // Early check outside lock, for quick return.

        if (mVibratorInfoLoadSuccessful) {

            return;

        }

        synchronized (mLock) {

            if (mVibratorInfoLoadSuccessful) {

                return;
@@ -132,17 +137,13 @@ final class VibratorController { 


    /** Checks if the {@link VibratorInfo} was loaded from the vibrator hardware successfully. */

    boolean isVibratorInfoLoadSuccessful() {

        synchronized (mLock) {

        return mVibratorInfoLoadSuccessful;

    }

    }



    /** Return the {@link VibratorInfo} representing the vibrator controlled by this instance. */

    public VibratorInfo getVibratorInfo() {

        synchronized (mLock) {

        return mVibratorInfo;

    }

    }



    /**

     * Return {@code true} is this vibrator is currently vibrating, false otherwise.
@@ -151,10 +152,8 @@ final class VibratorController { 
     * automatically notified to any registered {@link IVibratorStateListener} on change.

     */

    public boolean isVibrating() {

        synchronized (mLock) {

        return mIsVibrating;

    }

    }



    /**

     * Returns the current amplitude the device is vibrating.
@@ -168,17 +167,13 @@ final class VibratorController { 
     * <p>If {@link #isVibrating()} is false then this will be zero.

     */

    public float getCurrentAmplitude() {

        synchronized (mLock) {

        return mCurrentAmplitude;

    }

    }



    /** Return {@code true} if this vibrator is under external control, false otherwise. */

    public boolean isUnderExternalControl() {

        synchronized (mLock) {

        return mIsUnderExternalControl;

    }

    }



    /**

     * Check against this vibrator capabilities.
@@ -187,15 +182,15 @@ final class VibratorController { 
     * @return true if this vibrator has this capability, false otherwise

     */

    public boolean hasCapability(long capability) {

        synchronized (mLock) {

        return mVibratorInfo.hasCapability(capability);

    }

    }



    /** Return {@code true} if the underlying vibrator is currently available, false otherwise. */

    public boolean isAvailable() {

        synchronized (mLock) {

            return mNativeWrapper.isAvailable();

        }

    }



    /**

     * Set the vibrator control to be external or not, based on given flag.
@@ -203,10 +198,10 @@ final class VibratorController { 
     * <p>This will affect the state of {@link #isUnderExternalControl()}.

     */

    public void setExternalControl(boolean externalControl) {

        synchronized (mLock) {

        if (!mVibratorInfo.hasCapability(IVibrator.CAP_EXTERNAL_CONTROL)) {

            return;

        }

        synchronized (mLock) {

            mIsUnderExternalControl = externalControl;

            mNativeWrapper.setExternalControl(externalControl);

        }
@@ -217,10 +212,10 @@ final class VibratorController { 
     * if given {@code effect} is {@code null}.

     */

    public void updateAlwaysOn(int id, @Nullable PrebakedSegment prebaked) {

        synchronized (mLock) {

        if (!mVibratorInfo.hasCapability(IVibrator.CAP_ALWAYS_ON_CONTROL)) {

            return;

        }

        synchronized (mLock) {

            if (prebaked == null) {

                mNativeWrapper.alwaysOnDisable(id);

            } else {
@@ -256,7 +251,7 @@ final class VibratorController { 
            long duration = mNativeWrapper.on(milliseconds, vibrationId);

            if (duration > 0) {

                mCurrentAmplitude = -1;

                notifyVibratorOnLocked();

                notifyListenerOnVibrating(true);

            }

            return duration;

        }
@@ -277,7 +272,7 @@ final class VibratorController { 
                    prebaked.getEffectStrength(), vibrationId);

            if (duration > 0) {

                mCurrentAmplitude = -1;

                notifyVibratorOnLocked();

                notifyListenerOnVibrating(true);

            }

            return duration;

        }
@@ -293,14 +288,14 @@ final class VibratorController { 
     * do not support the input or a negative number if the operation failed.

     */

    public long on(PrimitiveSegment[] primitives, long vibrationId) {

        synchronized (mLock) {

        if (!mVibratorInfo.hasCapability(IVibrator.CAP_COMPOSE_EFFECTS)) {

            return 0;

        }

        synchronized (mLock) {

            long duration = mNativeWrapper.compose(primitives, vibrationId);

            if (duration > 0) {

                mCurrentAmplitude = -1;

                notifyVibratorOnLocked();

                notifyListenerOnVibrating(true);

            }

            return duration;

        }
@@ -315,15 +310,15 @@ final class VibratorController { 
     * @return The duration of the effect playing, or 0 if unsupported.

     */

    public long on(RampSegment[] primitives, long vibrationId) {

        synchronized (mLock) {

        if (!mVibratorInfo.hasCapability(IVibrator.CAP_COMPOSE_PWLE_EFFECTS)) {

            return 0;

        }

        synchronized (mLock) {

            int braking = mVibratorInfo.getDefaultBraking();

            long duration = mNativeWrapper.composePwle(primitives, braking, vibrationId);

            if (duration > 0) {

                mCurrentAmplitude = -1;

                notifyVibratorOnLocked();

                notifyListenerOnVibrating(true);

            }

            return duration;

        }
@@ -334,7 +329,7 @@ final class VibratorController { 
        synchronized (mLock) {

            mNativeWrapper.off();

            mCurrentAmplitude = 0;

            notifyVibratorOffLocked();

            notifyListenerOnVibrating(false);

        }

    }
@@ -349,7 +344,6 @@ final class VibratorController { 


    @Override

    public String toString() {

        synchronized (mLock) {

        return "VibratorController{"

                + "mVibratorInfo=" + mVibratorInfo

                + ", mVibratorInfoLoadSuccessful=" + mVibratorInfoLoadSuccessful
@@ -360,40 +354,21 @@ final class VibratorController { 
                + mVibratorStateListeners.getRegisteredCallbackCount()

                + '}';

    }

    }



    @GuardedBy("mLock")

    private void notifyVibratorOnLocked() {

        if (!mIsVibrating) {

            mIsVibrating = true;

            notifyStateListenersLocked();

    private void notifyListenerOnVibrating(boolean isVibrating) {

        if (mIsVibrating != isVibrating) {

            mIsVibrating = isVibrating;

            // The broadcast method is safe w.r.t. register/unregister listener methods, but lock

            // is required here to guarantee delivery order.

            mVibratorStateListeners.broadcast(

                    listener -> notifyStateListener(listener, isVibrating));

        }

    }



    @GuardedBy("mLock")

    private void notifyVibratorOffLocked() {

        if (mIsVibrating) {

            mIsVibrating = false;

            notifyStateListenersLocked();

        }

    }



    @GuardedBy("mLock")

    private void notifyStateListenersLocked() {

        final int length = mVibratorStateListeners.beginBroadcast();

        try {

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

                notifyStateListenerLocked(mVibratorStateListeners.getBroadcastItem(i));

            }

        } finally {

            mVibratorStateListeners.finishBroadcast();

        }

    }



    @GuardedBy("mLock")

    private void notifyStateListenerLocked(IVibratorStateListener listener) {

    private void notifyStateListener(IVibratorStateListener listener, boolean isVibrating) {

        try {

            listener.onVibrating(mIsVibrating);

            listener.onVibrating(isVibrating);

        } catch (RemoteException | RuntimeException e) {

            Slog.e(TAG, "Vibrator state listener failed to call", e);

        }

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

+22 −1
Original line number Diff line number Diff line
@@ -391,6 +391,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub { 
            fillVibrationFallbacks(vib, effect);



            synchronized (mLock) {

                if (DEBUG) {

                    Slog.d(TAG, "Starting vibrate for vibration  " + vib.id);

                }

                Vibration.Status ignoreStatus = shouldIgnoreVibrationLocked(vib);

                if (ignoreStatus != null) {

                    endVibrationLocked(vib, ignoreStatus);
@@ -498,6 +501,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub { 
    @VisibleForTesting

    void updateServiceState() {

        synchronized (mLock) {

            if (DEBUG) {

                Slog.d(TAG, "Updating device state...");

            }

            boolean inputDevicesChanged = mInputDeviceDelegate.updateInputDeviceVibrators(

                    mVibrationSettings.shouldVibrateInputDevices());
@@ -611,6 +617,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub { 
        Trace.asyncTraceEnd(Trace.TRACE_TAG_VIBRATOR, "vibration", 0);

        try {

            Vibration vib = mCurrentVibration.getVibration();

            if (DEBUG) {

                Slog.d(TAG, "Reporting vibration " + vib.id + " finished with status " + status);

            }

            endVibrationLocked(vib, status);

            finishAppOpModeLocked(vib.uid, vib.opPkg);

        } finally {
@@ -1062,11 +1071,17 @@ public class VibratorManagerService extends IVibratorManagerService.Stub { 
                Slog.d(TAG, "Vibrators released after finished vibration");

            }

            synchronized (mLock) {

                if (DEBUG) {

                    Slog.d(TAG, "Processing vibrators released callback");

                }

                mCurrentVibration = null;

                if (mNextVibration != null) {

                    VibrationThread vibThread = mNextVibration;

                    mNextVibration = null;

                    startVibrationThreadLocked(vibThread);

                    Vibration.Status status = startVibrationThreadLocked(vibThread);

                    if (status != Vibration.Status.RUNNING) {

                        endVibrationLocked(vibThread.getVibration(), status);

                    }

                }

            }

        }
@@ -1248,6 +1263,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub { 
        void dumpText(PrintWriter pw) {

            pw.println("Vibrator Manager Service:");

            synchronized (mLock) {

                if (DEBUG) {

                    Slog.d(TAG, "Dumping vibrator manager service to text...");

                }

                pw.println("  mVibrationSettings:");

                pw.println("    " + mVibrationSettings);

                pw.println();
@@ -1290,6 +1308,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub { 
            final ProtoOutputStream proto = new ProtoOutputStream(fd);



            synchronized (mLock) {

                if (DEBUG) {

                    Slog.d(TAG, "Dumping vibrator manager service to proto...");

                }

                mVibrationSettings.dumpProto(proto);

                if (mCurrentVibration != null) {

                    mCurrentVibration.getVibration().getDebugInfo().dumpProto(proto,