Skip to content

Commit 38e8052

Browse files
committed
fix(server): clean up ClientManager & prevent IndexOutOfBoundsException
While the ClientManager is shutting down, other shut-down processes such as the GameRoom close can lead to clients being removed from another thread. Thus the ClientManager close method needs multiple fail-safes. # Conflicts: # server/src/sc/server/Lobby.kt # server/src/sc/server/network/ClientManager.java # server/src/sc/server/network/NewClientListener.java
1 parent ccf7410 commit 38e8052

File tree

1 file changed

+23
-21
lines changed

1 file changed

+23
-21
lines changed

server/src/sc/server/network/ClientManager.kt

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ class ClientManager : Runnable, IClientListener {
1717
private val clientListener = NewClientListener()
1818

1919
private var running: Boolean = false
20-
private var thread: Thread? = null
20+
private var serviceThread: Thread? = null
2121

2222
private var onClientConnected: ((Client) -> Unit)? = null
2323

2424
init {
25-
this.running = false
26-
this.thread = null
25+
running = false
26+
serviceThread = null
2727
}
2828

2929
/**
@@ -32,7 +32,7 @@ class ClientManager : Runnable, IClientListener {
3232
* *(only used by tests and addAll())*
3333
*/
3434
fun add(newClient: Client) {
35-
this.clients.add(newClient)
35+
clients.add(newClient)
3636
newClient.addClientListener(this)
3737
onClientConnected?.invoke(newClient)
3838
}
@@ -43,28 +43,25 @@ class ClientManager : Runnable, IClientListener {
4343

4444
/** Fetch new clients */
4545
override fun run() {
46-
this.running = true
46+
running = true
4747

4848
logger.info("ClientManager running")
4949

50-
while(this.running && !Thread.interrupted()) {
50+
while(running && !Thread.interrupted()) {
5151
try {
5252
// Waits blocking for new Client
53-
val client = this.clientListener.fetchNewSingleClient()
53+
val client = clientListener.fetchNewSingleClient()
5454

5555
logger.info("Delegating new client to ClientManager...")
56-
this.add(client)
56+
add(client)
5757
logger.info("Delegation done")
5858
} catch(e: InterruptedException) {
59-
if(this.running) {
59+
if(running)
6060
logger.warn("Interrupted while waiting for a new client", e)
61-
} else {
62-
logger.warn("Client manager is shutting down")
63-
}
6461
}
6562
}
6663

67-
this.running = false
64+
running = false
6865
logger.info("ClientManager closed")
6966
}
7067

@@ -75,17 +72,22 @@ class ClientManager : Runnable, IClientListener {
7572
*/
7673
@Throws(IOException::class)
7774
fun start() {
78-
this.clientListener.start()
79-
if(this.thread == null)
80-
this.thread = ServiceManager.createService(this.javaClass.simpleName, this).apply { start() }
75+
clientListener.start()
76+
if(serviceThread == null)
77+
serviceThread = ServiceManager.createService(javaClass.simpleName, this).apply { start() }
8178
}
8279

8380
fun close() {
84-
this.running = false
85-
this.thread?.interrupt()
86-
this.clientListener.close()
87-
// Stop clients in reverse order to ensure all clients are stopped since they might be removed by onClientDisconnected
88-
this.clients.indices.reversed().forEach { clients[it].stop() }
81+
running = false
82+
serviceThread?.interrupt()
83+
clientListener.close()
84+
while (clients.size > 0) {
85+
try {
86+
clients.removeAt(clients.lastIndex).stop()
87+
} catch (ignored: ArrayIndexOutOfBoundsException) {
88+
// Client was removed concurrently
89+
}
90+
}
8991
}
9092

9193
/**

0 commit comments

Comments
 (0)