Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source
This commit is contained in:
Simon Bowden
2021-08-05 16:11:04 +00:00
committed by Android (Google) Code Review
parent bf1835d2ea 7ab12ba890
commit 47fc76e4f2
2 changed files with 18 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
services/core/java/com/android/server/vibrator/VibrationThread.java
View File

@@ -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 {
synchronized (mLock) {
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();
if (!mPendingOnVibratorCompleteSteps.isEmpty()) {
// Steps anticipated by vibrator complete callback should be played right away.
return 0;
}
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

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

@@ -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";