From 1c34029e872e3527a31dcd00ed476ed5d484cf1a Mon Sep 17 00:00:00 2001
From: olly <olly@google.com>
Date: Tue, 28 Apr 2020 14:59:23 +0100
Subject: [PATCH] ConditionVariable: Fix block(long) to correctly handle large
 timeouts

PiperOrigin-RevId: 308815613
---
 .../exoplayer2/util/ConditionVariable.java    | 25 +++++++++++-----
 .../util/ConditionVariableTest.java           | 29 ++++++++++++++++++-
 .../exoplayer2/testutil/TestUtilTest.java     |  4 +--
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/ConditionVariable.java b/library/core/src/main/java/com/google/android/exoplayer2/util/ConditionVariable.java
index 5628b35780..b7f0d04e23 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/util/ConditionVariable.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/util/ConditionVariable.java
@@ -86,18 +86,27 @@ public class ConditionVariable {
   }
 
   /**
-   * Blocks until the condition is opened or until {@code timeout} milliseconds have passed.
+   * Blocks until the condition is opened or until {@code timeoutMs} have passed.
    *
-   * @param timeout The maximum time to wait in milliseconds.
+   * @param timeoutMs The maximum time to wait in milliseconds. If {@code timeoutMs <= 0} then the
+   *     call will return immediately without blocking.
    * @return True if the condition was opened, false if the call returns because of the timeout.
    * @throws InterruptedException If the thread is interrupted.
    */
-  public synchronized boolean block(long timeout) throws InterruptedException {
-    long now = clock.elapsedRealtime();
-    long end = now + timeout;
-    while (!isOpen && now < end) {
-      wait(end - now);
-      now = clock.elapsedRealtime();
+  public synchronized boolean block(long timeoutMs) throws InterruptedException {
+    if (timeoutMs <= 0) {
+      return isOpen;
+    }
+    long nowMs = clock.elapsedRealtime();
+    long endMs = nowMs + timeoutMs;
+    if (endMs < nowMs) {
+      // timeoutMs is large enough for (nowMs + timeoutMs) to rollover. Block indefinitely.
+      block();
+    } else {
+      while (!isOpen && nowMs < endMs) {
+        wait(endMs - nowMs);
+        nowMs = clock.elapsedRealtime();
+      }
     }
     return isOpen;
   }
diff --git a/library/core/src/test/java/com/google/android/exoplayer2/util/ConditionVariableTest.java b/library/core/src/test/java/com/google/android/exoplayer2/util/ConditionVariableTest.java
index 5f58b6cbed..8f2fb2ed14 100644
--- a/library/core/src/test/java/com/google/android/exoplayer2/util/ConditionVariableTest.java
+++ b/library/core/src/test/java/com/google/android/exoplayer2/util/ConditionVariableTest.java
@@ -43,7 +43,7 @@ public class ConditionVariableTest {
   public void blockWithTimeout_blocksForAtLeastTimeout() throws InterruptedException {
     ConditionVariable conditionVariable = buildTestConditionVariable();
     long startTimeMs = System.currentTimeMillis();
-    assertThat(conditionVariable.block(/* timeout= */ 500)).isFalse();
+    assertThat(conditionVariable.block(/* timeoutMs= */ 500)).isFalse();
     long endTimeMs = System.currentTimeMillis();
     assertThat(endTimeMs - startTimeMs).isAtLeast(500);
   }
@@ -75,6 +75,33 @@ public class ConditionVariableTest {
     assertThat(conditionVariable.isOpen()).isFalse();
   }
 
+  @Test
+  public void blockWithMaxTimeout_blocks() throws InterruptedException {
+    ConditionVariable conditionVariable = buildTestConditionVariable();
+
+    AtomicBoolean blockReturned = new AtomicBoolean();
+    AtomicBoolean blockWasInterrupted = new AtomicBoolean();
+    Thread blockingThread =
+        new Thread(
+            () -> {
+              try {
+                conditionVariable.block(/* timeoutMs= */ Long.MAX_VALUE);
+                blockReturned.set(true);
+              } catch (InterruptedException e) {
+                blockWasInterrupted.set(true);
+              }
+            });
+
+    blockingThread.start();
+    Thread.sleep(500);
+    assertThat(blockReturned.get()).isFalse();
+
+    blockingThread.interrupt();
+    blockingThread.join();
+    assertThat(blockWasInterrupted.get()).isTrue();
+    assertThat(conditionVariable.isOpen()).isFalse();
+  }
+
   @Test
   public void open_unblocksBlock() throws InterruptedException {
     ConditionVariable conditionVariable = buildTestConditionVariable();
diff --git a/testutils/src/test/java/com/google/android/exoplayer2/testutil/TestUtilTest.java b/testutils/src/test/java/com/google/android/exoplayer2/testutil/TestUtilTest.java
index a80d474f9b..0a999c4161 100644
--- a/testutils/src/test/java/com/google/android/exoplayer2/testutil/TestUtilTest.java
+++ b/testutils/src/test/java/com/google/android/exoplayer2/testutil/TestUtilTest.java
@@ -30,7 +30,7 @@ public class TestUtilTest {
   public void createRobolectricConditionVariable_blockWithTimeout_timesOut()
       throws InterruptedException {
     ConditionVariable conditionVariable = TestUtil.createRobolectricConditionVariable();
-    assertThat(conditionVariable.block(/* timeout= */ 1)).isFalse();
+    assertThat(conditionVariable.block(/* timeoutMs= */ 1)).isFalse();
     assertThat(conditionVariable.isOpen()).isFalse();
   }
 
@@ -39,7 +39,7 @@ public class TestUtilTest {
       throws InterruptedException {
     ConditionVariable conditionVariable = TestUtil.createRobolectricConditionVariable();
     long startTimeMs = System.currentTimeMillis();
-    assertThat(conditionVariable.block(/* timeout= */ 500)).isFalse();
+    assertThat(conditionVariable.block(/* timeoutMs= */ 500)).isFalse();
     long endTimeMs = System.currentTimeMillis();
     assertThat(endTimeMs - startTimeMs).isAtLeast(500);
   }