Get back into a game a locked phone dropped out of
A phone locked when its turn came round could not play once it was unlocked. The game was never wrong — the host still had the position and the seat was still the player's — but the only route to it was gone, and nothing rebuilt it. The board came back with the state it had before the turn arrived, and every tap on it went nowhere. A locked phone puts its Wi-Fi radio into power save, and a link carrying nothing between one turn and the next is exactly what that drops. Four things were missing, and all four are needed: The radio is held awake for as long as there is a game on, which stops most of these drops happening at all. FULL_HIGH_PERF rather than the LOW_LATENCY its deprecation notice points at: that one only applies with the screen on and the app in front, which is the case that worked. A link with nothing on it is pinged, and answered. TCP does not report a link that died this way — the read never returns and the next write goes into a socket that leads nowhere — so without traffic of its own the app could not tell a player who was thinking from one who was gone. Both ends have to speak it or a quiet link would be dropped as dead, hence the protocol bump; the lobby already turns a mismatch away with "update both devices". A write that fails now reports the link as broken instead of closing it as though this end had meant to. That silence was the reason a dead board answered nothing and said nothing about why. And a guest that loses its link dials the host again — which is still listening, on the same port, for the whole match — and is put back in the seat it never actually left, by the device id it is already known by. The table then resyncs, which is what fills the screen back in. Coming back to the front asks for the same thing, since a client draws nothing but what it was sent and anything pushed while the link was down was pushed at nothing. Sends are serialised while I am here: a message is a line, a write is three calls, and the heartbeat writes on its own schedule. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01GarwvAPvWHELNspro4foFY
This commit is contained in:
Claude
@@ -228,17 +228,78 @@ class MatchControllerNetworkTest {
|
||||
guestLeft.start()
|
||||
guestLeft.leave(Side.GUEST)
|
||||
guestLeft.settle()
|
||||
assertEquals("Guest left the game.", guestLeft.host.abandoned.value)
|
||||
assertEquals("The connection to Guest was lost.", guestLeft.host.abandoned.value)
|
||||
|
||||
val hostLeft = chess()
|
||||
hostLeft.start()
|
||||
hostLeft.leave(Side.HOST)
|
||||
hostLeft.settle()
|
||||
assertEquals("The host left the game.", hostLeft.client.abandoned.value)
|
||||
assertEquals("The connection to the host was lost.", hostLeft.client.abandoned.value)
|
||||
// Nothing more can be played on a table with no host.
|
||||
assertEquals(emptyList<ChessMove>(), hostLeft.client.legalMoves.value)
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Coming back
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* The bug this is here for: a phone locked when its turn came round could
|
||||
* not play once it was unlocked again.
|
||||
*
|
||||
* A locked phone puts its Wi-Fi radio to sleep, and a link carrying nothing
|
||||
* between one turn and the next is exactly what that drops. The game was
|
||||
* never wrong — the host still had the position and the seat was still the
|
||||
* player's — but the only route to it was gone, and nothing rebuilt it. The
|
||||
* board came back on unlocking with the state it had before the turn
|
||||
* arrived, and every tap on it went nowhere.
|
||||
*/
|
||||
@Test
|
||||
fun `a guest whose link died can play again once it is back`() = runBlocking {
|
||||
val table = chess()
|
||||
table.start()
|
||||
table.white("e2", "e4")
|
||||
// Twenty replies to 1. e4 — the guest is on the clock.
|
||||
assertEquals(20, table.client.legalMoves.value.size)
|
||||
|
||||
table.dropGuestLink()
|
||||
assertNotNull("A dead link has to stop the table", table.client.abandoned.value)
|
||||
assertNotNull(table.host.abandoned.value)
|
||||
|
||||
table.reconnectGuest()
|
||||
|
||||
assertNull("The table is playable again and must not say otherwise", table.client.abandoned.value)
|
||||
assertNull(table.host.abandoned.value)
|
||||
assertNotNull("The guest came back to an empty board", table.client.state.value)
|
||||
assertEquals(
|
||||
"The guest is back but has nothing it is allowed to play",
|
||||
20,
|
||||
table.client.legalMoves.value.size,
|
||||
)
|
||||
|
||||
// And a move made on the new link is a move that actually happens.
|
||||
table.black("e7", "e5")
|
||||
assertNotNull(table.host.state.value?.board?.get(squareFromName("e5")))
|
||||
assertNull(table.host.state.value?.board?.get(squareFromName("e7")))
|
||||
}
|
||||
|
||||
/**
|
||||
* A guest coming back must not be treated as a guest arriving. The host end
|
||||
* of the old link is the one that died; anything it has left to say about
|
||||
* itself belongs to it and not to the seat, which is occupied again.
|
||||
*/
|
||||
@Test
|
||||
fun `a guest coming back is not reported as having left`() = runBlocking {
|
||||
val table = chess()
|
||||
table.start()
|
||||
table.dropGuestLink()
|
||||
table.reconnectGuest()
|
||||
table.settle()
|
||||
|
||||
assertNull(table.host.abandoned.value)
|
||||
assertNull(table.client.abandoned.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `closing a link on purpose is not reported as the other player leaving`() = runBlocking {
|
||||
val table = chess()
|
||||
@@ -273,6 +334,11 @@ class MatchControllerNetworkTest {
|
||||
private val hostScope = CoroutineScope(Job() + Dispatchers.Default)
|
||||
private val clientScope = CoroutineScope(Job() + Dispatchers.Default)
|
||||
|
||||
/** Every socket a reconnection has opened since, so dispose can close them. */
|
||||
private val laterSockets = mutableListOf<Socket>()
|
||||
private val guestPeerId: String
|
||||
private val guestSeat: Int
|
||||
|
||||
val hostLink: StreamConnection
|
||||
private val clientLink: StreamConnection
|
||||
val host: MatchController<S, M>
|
||||
@@ -281,7 +347,8 @@ class MatchControllerNetworkTest {
|
||||
init {
|
||||
clientSocket.connect(InetSocketAddress("127.0.0.1", server.localPort), CONNECT_MILLIS)
|
||||
hostSocket = server.accept()
|
||||
val guestPeerId = hostSocket.inetAddress?.hostAddress ?: "guest"
|
||||
guestPeerId = hostSocket.inetAddress?.hostAddress ?: "guest"
|
||||
guestSeat = (0 until seatCount).first { it != hostSeat }
|
||||
|
||||
hostLink = StreamConnection(
|
||||
peerId = guestPeerId,
|
||||
@@ -332,11 +399,7 @@ class MatchControllerNetworkTest {
|
||||
)
|
||||
|
||||
host = controller(MatchController.Role.HOST, hostSeat, hostScope)
|
||||
client = controller(
|
||||
MatchController.Role.CLIENT,
|
||||
(0 until seatCount).first { it != hostSeat },
|
||||
clientScope,
|
||||
)
|
||||
client = controller(MatchController.Role.CLIENT, guestSeat, clientScope)
|
||||
|
||||
// The lobby is already reading the guest's link when the game starts,
|
||||
// and goes on reading it for the whole match. Without this the test
|
||||
@@ -372,6 +435,54 @@ class MatchControllerNetworkTest {
|
||||
settle()
|
||||
}
|
||||
|
||||
/**
|
||||
* The guest's link dies under it, as a sleeping radio drops one.
|
||||
*
|
||||
* The sockets are killed rather than the connections closed: a link
|
||||
* either end let go of on purpose is a player leaving, which is a
|
||||
* different thing entirely and is already tested above.
|
||||
*/
|
||||
suspend fun dropGuestLink() {
|
||||
runCatching { hostSocket.close() }
|
||||
runCatching { clientSocket.close() }
|
||||
settle()
|
||||
}
|
||||
|
||||
/**
|
||||
* The guest dials the host again, which is still listening on the same
|
||||
* port, and both ends are handed the new link — the host from its lobby
|
||||
* on the guest's Hello, the guest from its own once that has gone
|
||||
* through.
|
||||
*/
|
||||
suspend fun reconnectGuest() {
|
||||
val socket = Socket()
|
||||
socket.connect(InetSocketAddress("127.0.0.1", server.localPort), CONNECT_MILLIS)
|
||||
val accepted = server.accept()
|
||||
laterSockets += socket
|
||||
laterSockets += accepted
|
||||
|
||||
val newHostLink = StreamConnection(
|
||||
peerId = guestPeerId,
|
||||
kind = TransportKind.LAN,
|
||||
input = accepted.getInputStream(),
|
||||
output = accepted.getOutputStream(),
|
||||
scope = hostScope,
|
||||
onClosed = { runCatching { accepted.close() } },
|
||||
)
|
||||
val newClientLink = StreamConnection(
|
||||
peerId = "lan:host",
|
||||
kind = TransportKind.LAN,
|
||||
input = socket.getInputStream(),
|
||||
output = socket.getOutputStream(),
|
||||
scope = clientScope,
|
||||
onClosed = { runCatching { socket.close() } },
|
||||
)
|
||||
|
||||
host.rebindPeer(guestSeat, newHostLink)
|
||||
client.rebindHostLink(newClientLink)
|
||||
settle()
|
||||
}
|
||||
|
||||
/** Exactly what AppRoot.pop() and PlayScreen's onDispose do, in that order. */
|
||||
fun leave(side: Side) {
|
||||
if (side == Side.HOST) {
|
||||
@@ -392,6 +503,7 @@ class MatchControllerNetworkTest {
|
||||
fun dispose() {
|
||||
runCatching { hostSocket.close() }
|
||||
runCatching { clientSocket.close() }
|
||||
laterSockets.forEach { runCatching { it.close() } }
|
||||
runCatching { server.close() }
|
||||
runCatching { hostLink.close() }
|
||||
runCatching { clientLink.close() }
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
package org.prolibertate.games.net
|
||||
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.cancel
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import java.io.InputStream
|
||||
import java.io.OutputStream
|
||||
import java.net.InetSocketAddress
|
||||
import java.net.ServerSocket
|
||||
import java.net.Socket
|
||||
import java.util.Collections
|
||||
import java.util.concurrent.CountDownLatch
|
||||
|
||||
/**
|
||||
* The heartbeat, which exists because a link with nothing on it cannot be told
|
||||
* from a link that is no longer there.
|
||||
*
|
||||
* Between one turn of a card game and the next, nothing at all crosses the
|
||||
* socket. A phone whose screen has gone off puts its Wi-Fi radio into power
|
||||
* save and, given long enough, the link goes with it — and TCP does not say so:
|
||||
* the read simply never returns, and the next write disappears into a socket
|
||||
* that goes nowhere. So the transport talks to itself, and treats a peer that
|
||||
* has stopped answering as gone.
|
||||
*/
|
||||
class StreamConnectionHeartbeatTest {
|
||||
|
||||
private val scopes = mutableListOf<CoroutineScope>()
|
||||
private val closeables = mutableListOf<() -> Unit>()
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
closeables.forEach { runCatching { it() } }
|
||||
scopes.forEach { it.cancel() }
|
||||
}
|
||||
|
||||
private fun scope(): CoroutineScope =
|
||||
CoroutineScope(Job() + Dispatchers.Default).also { scopes += it }
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* The case the whole thing has to get right: two players thinking, nobody
|
||||
* moving, for far longer than a link is allowed to be silent. Both ends
|
||||
* answer each other's pings, so neither concludes the other has gone.
|
||||
*/
|
||||
@Test
|
||||
fun `a quiet link between two live peers is left alone`() = runBlocking {
|
||||
val server = ServerSocket(0)
|
||||
val clientSocket = Socket()
|
||||
closeables += { server.close() }
|
||||
closeables += { clientSocket.close() }
|
||||
|
||||
clientSocket.connect(InetSocketAddress("127.0.0.1", server.localPort), CONNECT_MILLIS)
|
||||
val hostSocket = server.accept()
|
||||
closeables += { hostSocket.close() }
|
||||
|
||||
val hostScope = scope()
|
||||
val clientScope = scope()
|
||||
val a = connection("host", hostSocket.getInputStream(), hostSocket.getOutputStream(), hostScope)
|
||||
val b = connection("guest", clientSocket.getInputStream(), clientSocket.getOutputStream(), clientScope)
|
||||
|
||||
val heardByA = subscribe(a, hostScope)
|
||||
val heardByB = subscribe(b, clientScope)
|
||||
|
||||
// Several times the silence allowance, with not one game message sent.
|
||||
delay(SILENCE_MILLIS * 4)
|
||||
|
||||
assertTrue("The host's end gave up on a peer that was answering", a.isOpen)
|
||||
assertTrue("The guest's end gave up on a peer that was answering", b.isOpen)
|
||||
// And the pings are the transport's own business: nothing above it sees
|
||||
// them, so a screen is not asked to redraw twice a second for nothing.
|
||||
assertEquals(emptyList<NetMessage>(), heardByA)
|
||||
assertEquals(emptyList<NetMessage>(), heardByB)
|
||||
}
|
||||
|
||||
/**
|
||||
* A link that has died without saying so. The reader would wait on it for
|
||||
* as long as the game lasted, which is what left a player tapping a board
|
||||
* that answered nothing.
|
||||
*/
|
||||
@Test
|
||||
fun `a peer that stops answering is given up on and reported`() = runBlocking {
|
||||
val scope = scope()
|
||||
val input = BlockingInputStream()
|
||||
closeables += { input.close() }
|
||||
|
||||
val link = connection("gone", input, DiscardingOutputStream(), scope)
|
||||
val heard = subscribe(link, scope)
|
||||
|
||||
// Long enough for the silence to be noticed, and then some.
|
||||
delay(SILENCE_MILLIS * 3)
|
||||
|
||||
assertFalse("A link nobody is answering on was left open", link.isOpen)
|
||||
assertEquals(
|
||||
"The screen was never told the link had gone",
|
||||
listOf<NetMessage>(Bye),
|
||||
heard,
|
||||
)
|
||||
}
|
||||
|
||||
/** A link this end let go of is not somebody else going away. */
|
||||
@Test
|
||||
fun `closing a link on purpose says nothing`() = runBlocking {
|
||||
val scope = scope()
|
||||
val input = BlockingInputStream()
|
||||
closeables += { input.close() }
|
||||
|
||||
val link = connection("ours", input, DiscardingOutputStream(), scope)
|
||||
val heard = subscribe(link, scope)
|
||||
|
||||
link.close()
|
||||
delay(SILENCE_MILLIS)
|
||||
|
||||
assertFalse(link.isOpen)
|
||||
assertEquals(emptyList<NetMessage>(), heard)
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
private fun connection(
|
||||
peerId: String,
|
||||
input: InputStream,
|
||||
output: OutputStream,
|
||||
scope: CoroutineScope,
|
||||
) = StreamConnection(
|
||||
peerId = peerId,
|
||||
kind = TransportKind.LAN,
|
||||
input = input,
|
||||
output = output,
|
||||
scope = scope,
|
||||
heartbeatMillis = HEARTBEAT_MILLIS,
|
||||
silenceMillis = SILENCE_MILLIS,
|
||||
)
|
||||
|
||||
/** Everything a screen would have been shown, in order. */
|
||||
private fun subscribe(link: Connection, scope: CoroutineScope): List<NetMessage> {
|
||||
val heard = Collections.synchronizedList(mutableListOf<NetMessage>())
|
||||
scope.launch { link.incoming.collect { heard += it } }
|
||||
return heard
|
||||
}
|
||||
|
||||
/**
|
||||
* A stream that has nothing to say and never will, which is what a link
|
||||
* whose far end has vanished looks like from here. Blocks until it is
|
||||
* closed, then fails the read the way a closed socket does.
|
||||
*/
|
||||
private class BlockingInputStream : InputStream() {
|
||||
private val gate = CountDownLatch(1)
|
||||
|
||||
override fun read(): Int {
|
||||
gate.await()
|
||||
throw java.io.IOException("stream closed")
|
||||
}
|
||||
|
||||
override fun close() = gate.countDown()
|
||||
}
|
||||
|
||||
private class DiscardingOutputStream : OutputStream() {
|
||||
override fun write(b: Int) = Unit
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val CONNECT_MILLIS = 2_000
|
||||
|
||||
/**
|
||||
* The real intervals are seconds; these are the same arrangement run
|
||||
* fast, so the tests take a moment rather than a minute.
|
||||
*/
|
||||
const val HEARTBEAT_MILLIS = 40L
|
||||
const val SILENCE_MILLIS = 200L
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user