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

Commit 13ff2a17 authored by Lais Andrade's avatar Lais Andrade
Browse files

Fix concurrency of IVibratorCallback 

Fix error when VibrationThread gets a callback from a previous vibrate
request after turning the vibrator off and on again.

This can be triggered by playing a repeating waveform with a very short
off step, for example with timings [10, 1000, 10, 1000]. One of the 1s
ON steps will be played with the vibrator off because the thread will
get the callback from a previous on() call after the vibrator was turned
off and on again.

Test: atest com.android.server.vibrator
Fix: 290585608
Change-Id: Ib790059704fdb86b2813e2c3d155cf71934cad05
Flag: NONE normal/day-to-day bugfix
parent 2cd1f3b2

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

+19 −8
Original line number Diff line number Diff line
@@ -56,10 +56,17 @@ final class VibratorController { 
    private volatile boolean mIsUnderExternalControl;

    private volatile float mCurrentAmplitude;



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

    /**

     * Listener for vibration completion callbacks from native.

     *

     * <p>Only the latest active native call to {@link VibratorController#on} will ever trigger this

     * completion callback, to avoid race conditions during a vibration playback. If a new call to

     * {@link #on} or {@link #off} happens before a previous callback was triggered then the

     * previous callback will be disabled, even if the new command fails.

     */

    public interface OnVibrationCompleteListener {



        /** Callback triggered when vibration is complete. */

        /** Callback triggered when an active vibration command is complete. */

        void onComplete(int vibratorId, long vibrationId);

    }
@@ -235,7 +242,7 @@ final class VibratorController { 
    }



    /**

     * Turn on the vibrator for {@code milliseconds} time, using {@code vibrationId} or completion

     * Turn on the vibrator for {@code milliseconds} time, using {@code vibrationId} for completion

     * callback to {@link OnVibrationCompleteListener}.

     *

     * <p>This will affect the state of {@link #isVibrating()}.
@@ -255,7 +262,7 @@ final class VibratorController { 
    }



    /**

     * Plays predefined vibration effect, using {@code vibrationId} or completion callback to

     * Plays predefined vibration effect, using {@code vibrationId} for completion callback to

     * {@link OnVibrationCompleteListener}.

     *

     * <p>This will affect the state of {@link #isVibrating()}.
@@ -276,8 +283,8 @@ final class VibratorController { 
    }



    /**

     * Plays a composition of vibration primitives, using {@code vibrationId} or completion callback

     * to {@link OnVibrationCompleteListener}.

     * Plays a composition of vibration primitives, using {@code vibrationId} for completion

     * callback to {@link OnVibrationCompleteListener}.

     *

     * <p>This will affect the state of {@link #isVibrating()}.

     *
@@ -299,7 +306,7 @@ final class VibratorController { 
    }



    /**

     * Plays a composition of pwle primitives, using {@code vibrationId} or completion callback

     * Plays a composition of pwle primitives, using {@code vibrationId} for completion callback

     * to {@link OnVibrationCompleteListener}.

     *

     * <p>This will affect the state of {@link #isVibrating()}.
@@ -321,7 +328,11 @@ final class VibratorController { 
        }

    }



    /** Turns off the vibrator. This will affect the state of {@link #isVibrating()}. */

    /**

     * Turns off the vibrator and disables completion callback to any pending vibration.

     *

     * <p>This will affect the state of {@link #isVibrating()}.

     */

    public void off() {

        synchronized (mLock) {

            mNativeWrapper.off();

services/core/jni/com_android_server_vibrator_VibratorController.cpp

+13 −1
Original line number Diff line number Diff line
@@ -131,17 +131,28 @@ public: 
    }



    std::function<void()> createCallback(jlong vibrationId) {

        return [vibrationId, this]() {

        auto callbackId = ++mCallbackId;

        return [vibrationId, callbackId, this]() {

            auto currentCallbackId = mCallbackId.load();

            if (currentCallbackId != callbackId) {

                // This callback is from an older HAL call that is no longer relevant to the service

                return;

            }

            auto jniEnv = GetOrAttachJNIEnvironment(sJvm);

            jniEnv->CallVoidMethod(mCallbackListener, sMethodIdOnComplete, mVibratorId,

                                   vibrationId);

        };

    }



    void disableOldCallbacks() {

        mCallbackId++;

    }



private:

    const std::shared_ptr<vibrator::HalController> mHal;

    const int32_t mVibratorId;

    const jobject mCallbackListener;

    std::atomic<int64_t> mCallbackId;

};



static aidl::BrakingPwle brakingPwle(aidl::Braking braking, int32_t duration) {
@@ -236,6 +247,7 @@ static void vibratorOff(JNIEnv* env, jclass /* clazz */, jlong ptr) { 
    }

    auto offFn = [](vibrator::HalWrapper* hal) { return hal->off(); };

    wrapper->halCall<void>(offFn, "off");

    wrapper->disableOldCallbacks();

}



static void vibratorSetAmplitude(JNIEnv* env, jclass /* clazz */, jlong ptr, jfloat amplitude) {