From 45d683468b9ac7cd5139865c0bda92d7d5f43505 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Fri, 15 Dec 2023 22:52:35 -0500 Subject: [PATCH] Implement AsyncTimeout.cancel() (#1396) * Implement AsyncTimeout.cancel() This is a simple implementation that uses 3 booleans to keep state (inQueue, isCanceled, hadTimeoutWhenCanceled). I'd like to do a follow-up change to replace 3 booleans with a proper state variable, especially since that eliminates some impossible states (like inQueue and isCanceled). * apiDump --- okio/api/okio.api | 1 + okio/src/jvmMain/kotlin/okio/AsyncTimeout.kt | 56 ++++++--- .../jvmTest/kotlin/okio/AsyncTimeoutTest.kt | 115 ++++++++++++++++++ 3 files changed, 155 insertions(+), 17 deletions(-) diff --git a/okio/api/okio.api b/okio/api/okio.api index c2195cda8b..176a275bf4 100644 --- a/okio/api/okio.api +++ b/okio/api/okio.api @@ -47,6 +47,7 @@ public class okio/AsyncTimeout : okio/Timeout { public static final field Companion Lokio/AsyncTimeout$Companion; public fun ()V public final fun access$newTimeoutException (Ljava/io/IOException;)Ljava/io/IOException; + public fun cancel ()V public final fun enter ()V public final fun exit ()Z protected fun newTimeoutException (Ljava/io/IOException;)Ljava/io/IOException; diff --git a/okio/src/jvmMain/kotlin/okio/AsyncTimeout.kt b/okio/src/jvmMain/kotlin/okio/AsyncTimeout.kt index 87d87d774e..d32fa24269 100644 --- a/okio/src/jvmMain/kotlin/okio/AsyncTimeout.kt +++ b/okio/src/jvmMain/kotlin/okio/AsyncTimeout.kt @@ -48,6 +48,9 @@ open class AsyncTimeout : Timeout() { /** If scheduled, this is the time that the watchdog should time this out. */ private var timeoutAt = 0L + private var isCanceled = false + private var hadTimeoutWhenCanceled = false + fun enter() { val timeoutNanos = timeoutNanos() val hasDeadline = hasDeadline() @@ -59,7 +62,28 @@ open class AsyncTimeout : Timeout() { /** Returns true if the timeout occurred. */ fun exit(): Boolean { - return cancelScheduledTimeout(this) + lock.withLock { + if (isCanceled) { + return hadTimeoutWhenCanceled + .also { + isCanceled = false + hadTimeoutWhenCanceled = false + } + } + + return cancelScheduledTimeout(this) + } + } + + override fun cancel() { + super.cancel() + + lock.withLock { + if (isCanceled) return + if (!inQueue) return + isCanceled = true + hadTimeoutWhenCanceled = cancelScheduledTimeout(this) + } } /** @@ -270,24 +294,22 @@ open class AsyncTimeout : Timeout() { /** Returns true if the timeout occurred. */ private fun cancelScheduledTimeout(node: AsyncTimeout): Boolean { - AsyncTimeout.lock.withLock { - if (!node.inQueue) return false - node.inQueue = false - - // Remove the node from the linked list. - var prev = head - while (prev != null) { - if (prev.next === node) { - prev.next = node.next - node.next = null - return false - } - prev = prev.next + if (!node.inQueue) return false + node.inQueue = false + + // Remove the node from the linked list. + var prev = head + while (prev != null) { + if (prev.next === node) { + prev.next = node.next + node.next = null + return false } - - // The node wasn't found in the linked list: it must have timed out! - return true + prev = prev.next } + + // The node wasn't found in the linked list: it must have timed out! + return true } /** diff --git a/okio/src/jvmTest/kotlin/okio/AsyncTimeoutTest.kt b/okio/src/jvmTest/kotlin/okio/AsyncTimeoutTest.kt index 64b141e39b..f3a90a2b16 100644 --- a/okio/src/jvmTest/kotlin/okio/AsyncTimeoutTest.kt +++ b/okio/src/jvmTest/kotlin/okio/AsyncTimeoutTest.kt @@ -372,9 +372,124 @@ class AsyncTimeoutTest { assertEquals(of(*data), target.readByteString()) } + @Test + fun enterCancelSleepExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + timeout.enter() + timeout.cancel() + Thread.sleep(500) + + // Call didn't time out because the timeout was canceled. + assertFalse(timeout.exit()) + assertTimedOut() + } + + @Test + fun enterCancelCancelSleepExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + timeout.enter() + timeout.cancel() + timeout.cancel() + Thread.sleep(500) + + // Call didn't time out because the timeout was canceled. + assertFalse(timeout.exit()) + assertTimedOut() + } + + @Test + fun enterSleepCancelExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + timeout.enter() + Thread.sleep(500) + timeout.cancel() + + // Call timed out because the cancel was too late. + assertTrue(timeout.exit()) + assertTimedOut(timeout) + } + + @Test + fun enterSleepCancelCancelExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + timeout.enter() + Thread.sleep(500) + timeout.cancel() + timeout.cancel() + + // Call timed out because both cancels were too late. + assertTrue(timeout.exit()) + assertTimedOut(timeout) + } + + @Test + fun enterCancelSleepCancelExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + timeout.enter() + timeout.cancel() + Thread.sleep(500) + timeout.cancel() + + // Call didn't time out because the timeout was canceled. + assertFalse(timeout.exit()) + assertTimedOut() + } + + @Test + fun cancelEnterSleepExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + timeout.cancel() + timeout.enter() + Thread.sleep(500) + + // Call timed out because the cancel was too early. + assertTrue(timeout.exit()) + assertTimedOut(timeout) + } + + @Test + fun cancelEnterCancelSleepExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + timeout.cancel() + timeout.enter() + timeout.cancel() + Thread.sleep(500) + + // Call didn't time out because the timeout was canceled. + assertFalse(timeout.exit()) + assertTimedOut() + } + + @Test + fun enterCancelSleepExitEnterSleepExit() { + val timeout = RecordingAsyncTimeout() + timeout.timeout(250, TimeUnit.MILLISECONDS) + + // First call doesn't time out because we cancel it. + timeout.enter() + timeout.cancel() + Thread.sleep(500) + assertFalse(timeout.exit()) + assertTimedOut() + + // Second call does time out because it isn't canceled a second time. + timeout.enter() + Thread.sleep(500) + assertTrue(timeout.exit()) + assertTimedOut(timeout) + } + /** Asserts which timeouts fired, and in which order. */ private fun assertTimedOut(vararg expected: Timeout) { assertEquals(expected.toList(), timedOut.toList()) + timedOut.clear() } internal inner class RecordingAsyncTimeout : AsyncTimeout() {