diff --git a/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java b/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java index 8ed002ae7ed39..b8885e81dbe26 100644 --- a/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java +++ b/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java @@ -189,44 +189,42 @@ final class StartSequentialEffectStep extends Step { // vibrating different sets of vibrators in parallel. The manager can only prepareSynced // one set of vibrators at a time. // This property is guaranteed by there only being one thread (VibrationThread) executing - // one Step at a time, so there's no need to hold the state lock. - // TODO: remove the large locked block in a dedicated change. - synchronized (conductor.mLock) { - boolean hasPrepared = false; - boolean hasTriggered = false; - long maxDuration = 0; - try { - hasPrepared = conductor.vibratorManagerHooks.prepareSyncedVibration( - effectMapping.getRequiredSyncCapabilities(), - effectMapping.getVibratorIds()); + // one Step at a time, so there's no need to hold the state lock. Callbacks will be + // delivered asynchronously but enqueued until the step processing is finished. + boolean hasPrepared = false; + boolean hasTriggered = false; + long maxDuration = 0; + try { + hasPrepared = conductor.vibratorManagerHooks.prepareSyncedVibration( + effectMapping.getRequiredSyncCapabilities(), + effectMapping.getVibratorIds()); - for (AbstractVibratorStep step : steps) { - long duration = startVibrating(step, nextSteps); - if (duration < 0) { - // One vibrator has failed, fail this entire sync attempt. - return maxDuration = -1; - } - maxDuration = Math.max(maxDuration, duration); + for (AbstractVibratorStep step : steps) { + long duration = startVibrating(step, nextSteps); + if (duration < 0) { + // One vibrator has failed, fail this entire sync attempt. + return maxDuration = -1; } + maxDuration = Math.max(maxDuration, duration); + } - // Check if sync was prepared and if any step was accepted by a vibrator, - // otherwise there is nothing to trigger here. - if (hasPrepared && maxDuration > 0) { - hasTriggered = conductor.vibratorManagerHooks.triggerSyncedVibration( - getVibration().id); - } - return maxDuration; - } finally { - if (hasPrepared && !hasTriggered) { - // Trigger has failed or all steps were ignored by the vibrators. - conductor.vibratorManagerHooks.cancelSyncedVibration(); - nextSteps.clear(); - } else if (maxDuration < 0) { - // Some vibrator failed without being prepared so other vibrators might be - // active. Cancel and remove every pending step from output list. - for (int i = nextSteps.size() - 1; i >= 0; i--) { - nextSteps.remove(i).cancelImmediately(); - } + // Check if sync was prepared and if any step was accepted by a vibrator, + // otherwise there is nothing to trigger here. + if (hasPrepared && maxDuration > 0) { + hasTriggered = conductor.vibratorManagerHooks.triggerSyncedVibration( + getVibration().id); + } + return maxDuration; + } finally { + if (hasPrepared && !hasTriggered) { + // Trigger has failed or all steps were ignored by the vibrators. + conductor.vibratorManagerHooks.cancelSyncedVibration(); + nextSteps.clear(); + } else if (maxDuration < 0) { + // Some vibrator failed without being prepared so other vibrators might be + // active. Cancel and remove every pending step from output list. + for (int i = nextSteps.size() - 1; i >= 0; i--) { + nextSteps.remove(i).cancelImmediately(); } } } diff --git a/services/core/java/com/android/server/vibrator/VibrationThread.java b/services/core/java/com/android/server/vibrator/VibrationThread.java index 1b8e4c5bbb6bc..f2cd8c3ec3f80 100644 --- a/services/core/java/com/android/server/vibrator/VibrationThread.java +++ b/services/core/java/com/android/server/vibrator/VibrationThread.java @@ -242,8 +242,9 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { } } // Only run the next vibration step if we didn't have to wait in this loop. - // If we waited then the queue may have changed, so loop again to re-evaluate - // the scheduling of the queue top element. + // If we waited then the queue may have changed or the wait could have been + // interrupted by a cancel call, so loop again to re-evaluate the scheduling of + // the queue top element. if (waitMillisBeforeNextStep <= 0) { if (DEBUG) { Slog.d(TAG, "Play vibration consuming next step...");