I'm developing a poker game server using JAVA, and I want to add reconnect feature into my game. The logic is as follow:
Whenever a player is disconnected while playing, his correspondent data object will be keep until the end of current game. If he can connect back (reconnect) before the game ends, he can use that data object to continue playing.
Here's my code:
////// file: Game.java
import java.util.ArrayList;
import java.util.List;
class Game{
private int id;
//Maintain the list of ids of players who are playing in this game
private List<Integer> gamePlayerIds = new ArrayList<>(9); //Max 9 players in a Poker table
//reference back to main class
private GameController mnt;
public Game(int id, GameController mnt){
this.id = id;
this.mnt = mnt;
}
public void endGame(){
//find players who are disconnected while playing and not yet reconnect
for (Integer playerId : gamePlayerIds){
GameController.Player player = mnt.getPlayerById(playerId);
if (player != null && !player.isConnected()){
//if found, remove player object from Hashmap to prevent Memory Leak
mnt.removePlayerById(playerId);
}
}
}
}
/////// file: GameController.java
import java.util.concurrent.ConcurrentHashMap;
public class GameController {
private ConcurrentHashMap<Integer, Player> players;
public Player getPlayerById(int id){
return players.get(id);
}
public void removePlayerById(int id){
players.remove(id);
}
public void addPlayer(Player player){
players.putIfAbsent(player.getId(), player);
}
public GameController(){
players = new ConcurrentHashMap<>();
/* Do other initializations here */
}
}
////////// file: Player.java
class Player{
private int id;
private boolean isConnected = true;
private boolean isPlaying = false;
public boolean isPlaying() {
return isPlaying;
}
public void setPlaying(boolean playing) {
isPlaying = playing;
}
public boolean isConnected() {
return isConnected;
}
public void setConnected(boolean connected) {
isConnected = connected;
}
public Player(int id){
this.id = id;
}
public int getId(){
return id;
}
}
//////// file: OnConnectEventHandler.java
class OnConnectEventHandler {
//reference back to main class
GameController mnt;
public OnConnectEventHandler(GameController mnt){
this.mnt = mnt;
}
/*
* Handle event when a connection is made. There're 3 cases:
* 1. New connection
* 2. Duplicated connection (already connect before, and not yet disconnect
* 3. Reconnect
*/
public void handleEvent(User user){
Player player = mnt.getPlayerById(user.getId());
if (player == null){
//New connection, then convert User to Player and add
//new player object to ConcurrentHashMap which maintains the list
//of online players
mnt.addPlayer(new Player(user.getId()));
} else {
if (player.isConnected()){
//TODO: Alert error because of duplicated login
return;
} else {
//set connected flag to true, so that the endGame function
//will not remove this reconnected user
player.setConnected(true);
}
}
}
}
///// file: OnDisconnectEventHandler
class OnDisconnectEventHandler {
//reference back to main class
GameController mnt;
public OnDisconnectEventHandler(GameController mnt){
this.mnt = mnt;
}
/*
Handle disconnect event, there are 2 cases:
1. Disconnected player is not playing, so remove its data immediately
2. Disconnected player is playing, so keep its data until the end of current game
so that if he reconnect before the game ends, he can continue to play that game
*/
public void handleEvent(User user){
Player player = mnt.getPlayerById(user.getId());
if (player != null){
if (player.isPlaying()){
//if player is disconnected while playing, just marked that he is disconnect instead of
//removing Player object immediately
player.setConnected(false);
} else {
//if player is not playing, remove Player object immediately to prevent memory leak
mnt.removePlayerById(user.getId());
}
}
}
}
I'm using a ConcurrentHashMap to maintain the list of online Players.
When a player is disconnect while playing, I set "disconnected" flag of that player to true (isDisconnected is a boolean attribute of Player class), instead of removing player object from Players hashmap.
When current game is ended, check all players in this game, if a player's disconnected flag is set to true, then remove that player from Players hashmap to prevent memory leak. (1)
When a player is connected, I check if the player object is existed on Players hashmap or not. If existed, and player's disconnected flag is set to true, then I set that flag to false, so that the step in (1) will not remove the player's data object. (2)
My implementation works fine on most case, but there's an issue here: In multi threads environment, code can be executed in following order:
(2): get existed player object from Players hashmap.
(1): player disconnected flag is still true, remove player object from Players hashmap.
(2): player disconnected flag is set to false; player object is only referenced from (2)'s thread memory. When (2) is finished, the reference is gone and player object is clear by GC process.
As a result, player is connected, but no player data is stored in program memory.
what should I do to fix that issue?
Thank you in advanced!
Update 1 Can I add synchronized block like this:
//in OnConnectEventHandler class
public void handleEvent(User user){
synchronized(mnt.players){
Player player = mnt.getPlayerById(user.getId());
if (player == null){
//New connection, then convert User to Player and add
//new player object to ConcurrentHashMap which maintains the list
//of online players
mnt.addPlayer(new Player(user.getId()));
} else {
if (player.isConnected()){
//TODO: Alert error because of duplicated login
return;
} else {
//set connected flag to true, so that the endGame function
//will not remove this reconnected user
player.setConnected(true);
}
}
}
}
// in OnDisconnectEventHandler class
public void handleEvent(User user){
synchronized(mnt.players){
Player player = mnt.getPlayerById(user.getId());
if (player != null){
if (player.isPlaying()){
//if player is disconnected while playing, just marked that he is disconnect instead of
//removing Player object immediately
player.setConnected(false);
} else {
//if player is not playing, remove Player object immediately to prevent memory leak
mnt.removePlayerById(user.getId());
}
}
}
}
If it works, does it causes performance issues? My game is a multiplayer game, which usually has 3k-5k CCU, and in the most crowded time, there was about 300 connect/disconnect event per second.
Update 2 I use separated threads to handle connect and disconnect events. The scenario in my question happens if a player reconnect at the same time when his current game ends.
A player can be disconnected when he is not in a game (e.g. in lobby, in private room...). In those cases, because player is not doing a "progressive" and "important" action, so I don't need to implement reconnect feature.
(1) and (2) need to synchronize on the players map, so that either of them, when having acquired the lock, sees a consistent view of the player map and players' states.