Skip to content

Commit

Permalink
Implement AsyncTimeout.cancel() (#1396)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
squarejesse authored Dec 16, 2023
1 parent d2d0b57 commit 45d6834
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 17 deletions.
1 change: 1 addition & 0 deletions okio/api/okio.api
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class okio/AsyncTimeout : okio/Timeout {
public static final field Companion Lokio/AsyncTimeout$Companion;
public fun <init> ()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;
Expand Down
56 changes: 39 additions & 17 deletions okio/src/jvmMain/kotlin/okio/AsyncTimeout.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
}
}

/**
Expand Down Expand Up @@ -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
}

/**
Expand Down
115 changes: 115 additions & 0 deletions okio/src/jvmTest/kotlin/okio/AsyncTimeoutTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 45d6834

Please sign in to comment.