Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConcurrentModificationException in the GameRoomManager #201

Closed
SKoschnicke opened this issue Feb 6, 2019 · 11 comments
Closed

ConcurrentModificationException in the GameRoomManager #201

SKoschnicke opened this issue Feb 6, 2019 · 11 comments
Assignees
Labels
action:implement Needs to be implemented, might take time
Milestone

Comments

@SKoschnicke
Copy link
Contributor

Ein Team ist beim Trainieren ihres Clients auf einen Fehler im Server gestossen. Der Server beendet sich mit folgender Fehlermeldung:

18:13:54 ERROR sc.networking.clients.XStreamClient - Unknown Communication Error
java.util.ConcurrentModificationException: null
        at java.util.HashMap$HashIterator.nextNode(HashMap.java:1442)
        at java.util.HashMap$ValueIterator.next(HashMap.java:1471)
        at java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1042)
        at sc.server.gaming.GameRoomManager.joinOrCreateGame(GameRoomManager.java:156)
        at sc.server.Lobby.onRequest(Lobby.kt:65)
        at sc.server.network.Client.notifyOnPacket(Client.java:93)
        at sc.server.network.Client.onObject(Client.java:230)
        at sc.networking.clients.XStreamClient.receiveThread(XStreamClient.java:115)
        at sc.networking.clients.XStreamClient$1.run(XStreamClient.java:77)
        at java.lang.Thread.run(Thread.java:748)
@SKoschnicke SKoschnicke added the bug label Feb 6, 2019
@xeruf
Copy link
Member

xeruf commented Feb 6, 2019

Das Problem erscheint mir, dass zwar alle einzelnen Methoden im GameRoomManager synchronized sind, aber es können immer noch zwei verschiedene Methoden gleichzeitig den state verändern.
Ich würde fast sagen, dass der GameRoomManager auf seinem eigenen Thread operieren sollte, sodass immer nur eine Sache gleichzeitig passiert.

@SKoschnicke
Copy link
Contributor Author

Benutzt der Server denn ueberhaupt mehrere Threads?

@xeruf
Copy link
Member

xeruf commented Feb 6, 2019

Naja, wahrscheinlich um mit mehreren Clients gleichzeitig kommunizieren zu können. Damit der obige Fehler passiert, muss irgendwo ein Thread die rooms verändert haben während der Iterator aktiv war.

@SKoschnicke
Copy link
Contributor Author

Ja, klingt plausibel. Dann also den GameRoomManager in einem eigenen Thread und die anderen Threads kommunizieren per message passing damit? Klingt nach einer groesseren Aenderung...

@xeruf
Copy link
Member

xeruf commented Feb 6, 2019

Jup, hab ich so gedacht. Ja, das ist ne größere Änderung, aber ich weiß nicht wie man es sonst wirklich sicher machen kann.

@xeruf
Copy link
Member

xeruf commented Feb 6, 2019

Vielleicht können wir da auch ein Kotlin pattern verwenden: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.channels/actor.html
Bzw hier eine bessere Introduction: https://medium.com/@jagsaund/kotlin-concurrency-with-actors-34bd12531182
Ein Actor ist so ziemlich das wovon wir bisher geredet haben. Er empfängt Nachrichten und sendet welche aus. Und die return value von so ziemlich jeder an die Clients gerichteten Methode ist ja eine Message.

@SKoschnicke
Copy link
Contributor Author

Ich bin mir nicht sicher, aber ich glaube, dass der GameRoomManager gar nicht direkt mit den Clients kommuniziert. Aber den koennten wir als Actor modellieren, ja.

@xeruf
Copy link
Member

xeruf commented Feb 6, 2019

Naja indirekt über die Lobby halt. Aber die leitet auch nur die Nachrichten hin und die Antworten zurück ^^
image

@SKoschnicke
Copy link
Contributor Author

Und die Lobby existiert doch auch nur einmal im Server.

2019-02-06-103346_1487x1008

@xeruf
Copy link
Member

xeruf commented Feb 6, 2019

Und die Lobby existiert doch auch nur einmal im Server.

Was soll uns das sagen?

Ich frage mich auch gerade, wie viel Sinn es überhaupt macht den GameManager und ClientManager von der Lobby zu separieren, weil die ja trotzdem ziemlich eng gecoupled sind.

@SKoschnicke
Copy link
Contributor Author

Und die Lobby existiert doch auch nur einmal im Server.

Was soll uns das sagen?

Ich dachte, wenn die Lobby nur einmal existiert, koennen da ja nicht mehrere Aufrufe parallel durch. Aber es geht darum, dass von mehreren Threads durchaus parallel auf die eine Lobby zugegriffen werden kann. Verstehe.

Die hohe Separierung ist eine Altlast. In den meisten Faellen kann man da was zusammenlegen.

@xeruf xeruf self-assigned this Feb 6, 2019
@xeruf xeruf changed the title Concurrent modification exception ConcurrentModificationException im GameRoomManager May 17, 2019
@xeruf xeruf added this to the 19.3.0 milestone May 17, 2019
@xeruf xeruf removed this from the 19.3.0 milestone Oct 7, 2019
@xeruf xeruf removed the bug label Feb 4, 2020
@xeruf xeruf changed the title ConcurrentModificationException im GameRoomManager ConcurrentModificationException in the GameRoomManager Feb 4, 2020
@xeruf xeruf added action:implement Needs to be implemented, might take time and removed priority:low labels Feb 4, 2020
@xeruf xeruf added this to the long-term milestone Jan 29, 2021
@xeruf xeruf closed this as completed in f37ef62 Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:implement Needs to be implemented, might take time
Projects
None yet
2 participants