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

Commit 89abfb36 authored by Simon Bowden's avatar Simon Bowden Committed by Automerger Merge Worker
Browse files

Merge "Fix a race condition between calculateWaitTime and actually waiting.... 

Merge "Fix a race condition between calculateWaitTime and actually waiting. When the race was lost with a completion callback, the operation being waited upon may have been de-queued by the completion callback for immediate execution, but the thread was still proceeding with the wait." into sc-dev am: 47fc76e4

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15472895

Change-Id: I3ddb046efb61663ba115212063e311889bb7dc85
parents df4f102c 47fc76e4

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

+17 −13
Original line number Diff line number Diff line
@@ -228,17 +228,20 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { 


            Vibration.Status status = null;

            while (!mStepQueue.isEmpty()) {

                long waitTime = mStepQueue.calculateWaitTime();

                if (waitTime <= 0) {

                    mStepQueue.consumeNext();

                } else {

                long waitTime;

                synchronized (mLock) {

                    waitTime = mStepQueue.calculateWaitTime();

                    if (waitTime > 0) {

                        try {

                            mLock.wait(waitTime);

                        } catch (InterruptedException e) {

                        }

                    }

                }

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

                if (waitTime <= 0) {

                    mStepQueue.consumeNext();

                }

                Vibration.Status currentStatus = mStop ? Vibration.Status.CANCELLED

                        : mStepQueue.calculateVibrationStatus(sequentialEffectSize);

                if (status == null && currentStatus != Vibration.Status.RUNNING) {
@@ -387,15 +390,13 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { 
        }



        /** Returns the time in millis to wait before calling {@link #consumeNext()}. */

        @GuardedBy("mLock")

        public long calculateWaitTime() {

            Step nextStep;

            synchronized (mLock) {

            if (!mPendingOnVibratorCompleteSteps.isEmpty()) {

                // Steps anticipated by vibrator complete callback should be played right away.

                return 0;

            }

                nextStep = mNextSteps.peek();

            }

            Step nextStep = mNextSteps.peek();

            return nextStep == null ? 0 : nextStep.calculateWaitTime();

        }
@@ -603,7 +604,10 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { 
            return false;

        }



        /** Returns the time in millis to wait before playing this step. */

        /**

         * Returns the time in millis to wait before playing this step. This is performed

         * while holding the queue lock, so should not rely on potentially slow operations.

         */

        public long calculateWaitTime() {

            if (startTime == Long.MAX_VALUE) {

                // This step don't have a predefined start time, it's just marked to be executed

services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java

+1 −1
Original line number Diff line number Diff line
@@ -83,7 +83,7 @@ import java.util.stream.Collectors; 
@Presubmit

public class VibrationThreadTest {



    private static final int TEST_TIMEOUT_MILLIS = 1_000;

    private static final int TEST_TIMEOUT_MILLIS = 900;

    private static final int UID = Process.ROOT_UID;

    private static final int VIBRATOR_ID = 1;

    private static final String PACKAGE_NAME = "package";