Improved server's reconnection and drafts stability:

* draft: fixed miss or empty draft panels on reconnect;
* draft: fixed tourney freezes for richman drafts on disconnects;
* draft: fixed tourney freezes on rare use cases with bad connection;
This commit is contained in:
Oleg Agafonov 2025-04-18 09:38:52 +04:00
parent 5e27be4dfa
commit 30d44ce869
17 changed files with 201 additions and 133 deletions

View file

@ -401,7 +401,8 @@
}
if (!draftBooster.isEmptyGrid()) {
SessionHandler.setBoosterLoaded(draftId); // confirm to the server that the booster has been successfully loaded, otherwise the server will re-send the booster
// confirm to the server that the booster has been successfully loaded, otherwise the server will re-send the booster
SessionHandler.setBoosterLoaded(draftId);
if (pickNo != protectionPickNo && !protectionTimer.isRunning()) {
// Restart the protection timer.

View file

@ -72,7 +72,7 @@ public class CallbackClientImpl implements CallbackClient {
SwingUtilities.invokeLater(() -> {
try {
if (DebugUtil.NETWORK_SHOW_CLIENT_CALLBACK_MESSAGES_LOG) {
logger.info("message " + callback.getMessageId() + " - " + callback.getMethod().getType() + " - " + callback.getMethod());
logger.info(callback.getInfo());
}
// process bad connection (events can income in wrong order, so outdated data must be ignored)

View file

@ -99,4 +99,7 @@ public class ClientCallback implements Serializable {
return messageId;
}
public String getInfo() {
return String.format("message %d - %s - %s", this.getMessageId(), this.getMethod().getType(), this.getMethod());
}
}

View file

@ -13,13 +13,12 @@ import mage.interfaces.ServerState;
import mage.interfaces.callback.ClientCallback;
import mage.players.PlayerType;
import mage.players.net.UserData;
import mage.utils.CompressUtil;
import mage.util.ThreadUtils;
import mage.utils.CompressUtil;
import mage.view.*;
import org.apache.log4j.Logger;
import org.jboss.remoting.*;
import org.jboss.remoting.callback.Callback;
import org.jboss.remoting.callback.HandleCallbackException;
import org.jboss.remoting.callback.InvokerCallbackHandler;
import org.jboss.remoting.transport.bisocket.Bisocket;
import org.jboss.remoting.transport.socket.SocketWrapper;
@ -31,6 +30,7 @@ import java.lang.reflect.UndeclaredThrowableException;
import java.net.*;
import java.util.*;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;
/**
@ -47,6 +47,8 @@ public class SessionImpl implements Session {
private static final int SESSION_VALIDATOR_PING_PERIOD_SECS = 4;
private static final int SESSION_VALIDATOR_PING_TIMEOUT_SECS = 3;
private static final int CONNECT_WAIT_BEFORE_PROCESS_ANY_CALLBACKS_SECS = 3;
public static final String ADMIN_NAME = "Admin"; // if you change here then change in User too
public static final String KEEP_MY_OLD_SESSION = "keep_my_old_session"; // for disconnects without active session lose (keep tables/games)
@ -598,24 +600,43 @@ public class SessionImpl implements Session {
class CallbackHandler implements InvokerCallbackHandler {
final CopyOnWriteArrayList<ClientCallback> waitingCallbacks = new CopyOnWriteArrayList<>();
@Override
public void handleCallback(Callback callback) throws HandleCallbackException {
try {
public void handleCallback(Callback callback) {
// keep callbacks
ClientCallback clientCallback = (ClientCallback) callback.getCallbackObject();
waitingCallbacks.add(clientCallback);
// wait for client ready
// on connection client will receive all waiting callbacks from a server, e.g. started table, draft pick, etc
// but it's require to get server settings first (server state), e.g. for test mode
// possible bugs: hidden cheat button or enabled clicks protection in draft
// possible bugs:
// - hidden cheat button or enabled clicks protection in draft
// - miss dialogs like draft or game panels
// so wait for server state some time
if (serverState == null) {
ThreadUtils.sleep(1000);
ThreadUtils.sleep(CONNECT_WAIT_BEFORE_PROCESS_ANY_CALLBACKS_SECS * 1000);
if (serverState == null) {
logger.error("Can't receive server state before other data (possible reason: unstable network)");
logger.error("Can't receive server state before other data (possible reason: unstable network): "
+ clientCallback.getInfo());
}
}
client.onCallback((ClientCallback) callback.getCallbackObject());
// execute waiting queue
// client.onCallback must process and ignore outdated data inside
List<ClientCallback> executingCallbacks;
synchronized (waitingCallbacks) {
executingCallbacks = new ArrayList<>(waitingCallbacks);
executingCallbacks.sort(Comparator.comparingInt(ClientCallback::getMessageId));
waitingCallbacks.clear();
}
try {
executingCallbacks.forEach(client::onCallback);
} catch (Exception ex) {
logger.error("handleCallback error", ex);
}
}
}

View file

@ -73,6 +73,7 @@ public class Session {
private final ReentrantLock lock;
private final ReentrantLock callBackLock;
private String lastCallbackInfo = "";
public Session(ManagerFactory managerFactory, String sessionId, InvokerCallbackHandler callbackHandler) {
this.managerFactory = managerFactory;
@ -429,8 +430,10 @@ public class Session {
*/
public void fireCallback(final ClientCallback call) {
boolean lockSet = false; // TODO: research about locks, why it here? 2023-12-06
try {
if (valid && callBackLock.tryLock(50, TimeUnit.MILLISECONDS)) {
lastCallbackInfo = call.getInfo();
call.setMessageId(messageId.incrementAndGet());
lockSet = true;
Callback callback = new Callback(call);
@ -440,11 +443,14 @@ public class Session {
}
} catch (InterruptedException ex) {
// already sending another command (connection problem?)
// TODO: un-support multiple games/drafts at the same time?!?!?!?!
if (call.getMethod().equals(ClientCallbackMethod.GAME_INIT)
|| call.getMethod().equals(ClientCallbackMethod.START_GAME)) {
// it's ok use case, user has connection problem so can't send game init (see sendInfoAboutPlayersNotJoinedYetAndTryToFixIt)
// it's ok, possible use cases:
// - user has connection problem so can't send game init (see sendInfoAboutPlayersNotJoinedYetAndTryToFixIt)
} else {
logger.warn("SESSION LOCK, possible connection problem - fireCallback - userId: " + userId + " messageId: " + call.getMessageId(), ex);
logger.warn("SESSION LOCK, possible connection problem - fireCallback - userId: "
+ userId + ", prev call: " + lastCallbackInfo + ", current call: " + call.getInfo(), ex);
}
} catch (HandleCallbackException ex) {
// general error

View file

@ -4,6 +4,7 @@ import mage.cards.decks.Deck;
import mage.constants.ManaType;
import mage.constants.TableState;
import mage.game.Table;
import mage.game.draft.DraftPlayer;
import mage.game.result.ResultProtos;
import mage.game.tournament.TournamentPlayer;
import mage.interfaces.callback.ClientCallback;
@ -441,6 +442,15 @@ public class User {
ccDraftStarted(entry.getValue().getDraft().getTableId(), entry.getValue().getDraft().getId(), entry.getKey());
entry.getValue().init();
entry.getValue().update();
// on reconnect must resend booster data to new player
// TODO: there are possible rare race conditions and user can get draft panel without game info, miss tourney panel, etc
// TODO: client side - it must show active panel like current game or draft instead tourney panel
DraftPlayer draftPlayer = entry.getValue().getDraftPlayer();
if (draftPlayer != null) {
draftPlayer.setBoosterNotLoaded();
entry.getValue().getDraft().boosterSendingStart();
}
}
// active constructing

View file

@ -1,6 +1,7 @@
package mage.server.draft;
import mage.game.draft.Draft;
import mage.game.draft.DraftPlayer;
import mage.interfaces.callback.ClientCallback;
import mage.interfaces.callback.ClientCallbackMethod;
import mage.server.User;
@ -156,6 +157,10 @@ public class DraftSession {
return new DraftPickView(draft.getPlayer(playerId), timeout);
}
public DraftPlayer getDraftPlayer() {
return draft.getPlayer(playerId);
}
public Draft getDraft() {
return draft;
}

View file

@ -38,20 +38,17 @@ public class ThreadExecutorImpl implements ThreadExecutor {
*/
public ThreadExecutorImpl(ConfigSettings config) {
//callExecutor = Executors.newCachedThreadPool();
callExecutor = new CachedThreadPoolWithException();
((ThreadPoolExecutor) callExecutor).setKeepAliveTime(60, TimeUnit.SECONDS);
((ThreadPoolExecutor) callExecutor).allowCoreThreadTimeOut(true);
((ThreadPoolExecutor) callExecutor).setThreadFactory(new XmageThreadFactory(ThreadUtils.THREAD_PREFIX_CALL_REQUEST));
//gameExecutor = Executors.newFixedThreadPool(config.getMaxGameThreads());
gameExecutor = new FixedThreadPoolWithException(config.getMaxGameThreads());
((ThreadPoolExecutor) gameExecutor).setKeepAliveTime(60, TimeUnit.SECONDS);
((ThreadPoolExecutor) gameExecutor).allowCoreThreadTimeOut(true);
((ThreadPoolExecutor) gameExecutor).setThreadFactory(new XmageThreadFactory(ThreadUtils.THREAD_PREFIX_GAME));
//tourney = Executors.newFixedThreadPool(config.getMaxGameThreads() / GAMES_PER_TOURNEY_RATIO);
tourneyExecutor = new FixedThreadPoolWithException(config.getMaxGameThreads() / GAMES_PER_TOURNEY_RATIO);
tourneyExecutor = new FixedThreadPoolWithException(Math.min(2, config.getMaxGameThreads() / GAMES_PER_TOURNEY_RATIO));
((ThreadPoolExecutor) tourneyExecutor).setKeepAliveTime(60, TimeUnit.SECONDS);
((ThreadPoolExecutor) tourneyExecutor).allowCoreThreadTimeOut(true);
((ThreadPoolExecutor) tourneyExecutor).setThreadFactory(new XmageThreadFactory(ThreadUtils.THREAD_PREFIX_TOURNEY));

View file

@ -34,6 +34,7 @@ public class BoosterDraft extends DraftImpl {
}
boosterNum++;
}
this.boosterSendingEnd();
this.fireEndDraftEvent();
}

View file

@ -34,6 +34,7 @@ public interface Draft extends MageItem, Serializable {
int getCardNum();
boolean addPick(UUID playerId, UUID cardId, Set<UUID> hiddenCards);
void setBoosterLoaded(UUID playerID);
void boosterSendingStart();
void start();
boolean isStarted();
void setStarted();

View file

@ -18,7 +18,7 @@ import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
/**
* @author BetaSteward_at_googlemail.com
* @author BetaSteward_at_googlemail.com, JayDi85
*/
public abstract class DraftImpl implements Draft {
@ -36,7 +36,7 @@ public abstract class DraftImpl implements Draft {
protected int cardNum = 1; // starts with card number 1, increases by +1 after each picking
protected TimingOption timing;
protected int boosterLoadingCounter; // number of times the boosters have been sent to players until all are confirmed to have received them
protected final int BOOSTER_LOADING_INTERVAL = 2; // interval in seconds
protected final int BOOSTER_LOADING_INTERVAL_SECS = 2; // interval in seconds
protected boolean abort = false;
protected boolean started = false;
@ -44,8 +44,8 @@ public abstract class DraftImpl implements Draft {
protected transient TableEventSource tableEventSource = new TableEventSource();
protected transient PlayerQueryEventSource playerQueryEventSource = new PlayerQueryEventSource();
protected ScheduledFuture<?> boosterLoadingHandle;
protected ScheduledExecutorService boosterLoadingExecutor = null;
protected ScheduledFuture<?> boosterSendingWorker;
protected ScheduledExecutorService boosterSendingExecutor = null;
public DraftImpl(DraftOptions options, List<ExpansionSet> sets) {
this.id = UUID.randomUUID();
@ -83,7 +83,6 @@ public abstract class DraftImpl implements Draft {
if (newPlayer != null) {
DraftPlayer newDraftPlayer = new DraftPlayer(newPlayer);
DraftPlayer oldDraftPlayer = players.get(oldPlayer.getId());
newDraftPlayer.setBooster(oldDraftPlayer.getBooster());
Map<UUID, DraftPlayer> newPlayers = new LinkedHashMap<>();
synchronized (players) {
for (Map.Entry<UUID, DraftPlayer> entry : players.entrySet()) {
@ -110,12 +109,14 @@ public abstract class DraftImpl implements Draft {
table.setCurrent(currentId);
}
// boosters send to all players by timeout, so don't need to send it manually here
newDraftPlayer.setBoosterAndLoad(oldDraftPlayer.getBooster());
if (oldDraftPlayer.isPicking()) {
newDraftPlayer.setPicking();
if (!newDraftPlayer.getBooster().isEmpty()) {
newDraftPlayer.getPlayer().pickCard(newDraftPlayer.getBooster(), newDraftPlayer.getDeck(), this);
}
newDraftPlayer.setPickingAndSending();
}
boosterSendingStart(); // if it's AI then make pick from it
return true;
}
return false;
@ -124,7 +125,7 @@ public abstract class DraftImpl implements Draft {
@Override
public Collection<DraftPlayer> getPlayers() {
synchronized (players) {
return players.values();
return new ArrayList<>(players.values());
}
}
@ -188,7 +189,7 @@ public abstract class DraftImpl implements Draft {
List<Card> currentBooster = current.booster;
while (true) {
List<Card> nextBooster = next.booster;
next.setBooster(currentBooster);
next.setBoosterAndLoad(currentBooster);
if (Objects.equals(nextId, startId)) {
break;
}
@ -209,7 +210,7 @@ public abstract class DraftImpl implements Draft {
List<Card> currentBooster = current.booster;
while (true) {
List<Card> prevBooster = prev.booster;
prev.setBooster(currentBooster);
prev.setBoosterAndLoad(currentBooster);
if (Objects.equals(prevId, startId)) {
break;
}
@ -221,70 +222,71 @@ public abstract class DraftImpl implements Draft {
}
protected void openBooster() {
synchronized (players) {
if (boosterNum <= numberBoosters) {
for (DraftPlayer player : players.values()) {
if (draftCube != null) {
player.setBooster(draftCube.createBooster());
player.setBoosterAndLoad(draftCube.createBooster());
} else {
player.setBooster(sets.get(boosterNum - 1).createBooster());
player.setBoosterAndLoad(sets.get(boosterNum - 1).createBooster());
}
}
}
}
}
protected boolean pickCards() {
synchronized (players) {
for (DraftPlayer player : players.values()) {
if (player.getBooster().isEmpty()) {
return false;
}
player.setPicking();
player.setBoosterNotLoaded();
player.setPickingAndSending();
}
setupBoosterLoadingHandle();
synchronized (this) {
}
while (!donePicking()) {
try {
this.wait(10000); // checked every 10s to make sure the draft moves on
} catch (InterruptedException ex) {
}
}
boosterSendingStart();
picksWait();
}
cardNum++;
return true;
}
protected void setupBoosterLoadingHandle() {
cancelBoosterLoadingHandle();
boosterLoadingCounter = 0;
if (this.boosterLoadingExecutor == null) {
this.boosterLoadingExecutor = Executors.newSingleThreadScheduledExecutor(
new XmageThreadFactory(ThreadUtils.THREAD_PREFIX_TOURNEY_BOOSTERS_SEND)
public void boosterSendingStart() {
if (this.boosterSendingExecutor == null) {
this.boosterSendingExecutor = Executors.newSingleThreadScheduledExecutor(
new XmageThreadFactory(ThreadUtils.THREAD_PREFIX_TOURNEY_BOOSTERS_SEND + " " + this.getId())
);
}
boosterLoadingCounter = 0;
boosterLoadingHandle = boosterLoadingExecutor.scheduleAtFixedRate(() -> {
if (boosterSendingWorker == null) {
boosterSendingWorker = boosterSendingExecutor.scheduleAtFixedRate(() -> {
try {
if (loadBoosters()) {
cancelBoosterLoadingHandle();
if (isAbort() || sendBoostersToPlayers()) {
boosterSendingEnd();
} else {
boosterLoadingCounter++;
}
} catch (Exception ex) {
logger.fatal("Fatal boosterLoadingHandle error in draft " + id + " pack " + boosterNum + " pick " + cardNum, ex);
}
}, 0, BOOSTER_LOADING_INTERVAL, TimeUnit.SECONDS);
}
protected void cancelBoosterLoadingHandle() {
if (boosterLoadingHandle != null) {
boosterLoadingHandle.cancel(true);
}, 0, BOOSTER_LOADING_INTERVAL_SECS, TimeUnit.SECONDS);
}
}
protected boolean loadBoosters() {
protected void boosterSendingEnd() {
if (boosterSendingWorker != null) {
boosterSendingWorker.cancel(true);
boosterSendingWorker = null;
}
}
protected boolean sendBoostersToPlayers() {
boolean allBoostersLoaded = true;
for (DraftPlayer player : players.values()) {
for (DraftPlayer player : getPlayers()) {
if (player.isPicking() && !player.isBoosterLoaded()) {
allBoostersLoaded = false;
player.getPlayer().pickCard(player.getBooster(), player.getDeck(), this);
@ -297,17 +299,21 @@ public abstract class DraftImpl implements Draft {
if (isAbort()) {
return true;
}
synchronized (players) {
return players.values()
.stream()
.noneMatch(DraftPlayer::isPicking);
}
}
@Override
public boolean allJoined() {
synchronized (players) {
return players.values().stream()
.allMatch(DraftPlayer::isJoined);
}
}
@Override
public void addTableEventListener(Listener<TableEvent> listener) {
@ -342,11 +348,32 @@ public abstract class DraftImpl implements Draft {
// if the pack is re-sent to a player because they haven't been able to successfully load it, the pick time is reduced appropriately because of the elapsed time
// the time is always at least 1 second unless it's set to 0, i.e. unlimited time
if (time > 0) {
time = Math.max(1, time - boosterLoadingCounter * BOOSTER_LOADING_INTERVAL);
time = Math.max(1, time - boosterLoadingCounter * BOOSTER_LOADING_INTERVAL_SECS);
}
return time;
}
public void picksCheckDone() {
// notify main thread about changes, can be called from user's thread
synchronized (this) {
this.notifyAll();
}
}
protected void picksWait() {
// main thread waiting any picks or changes
synchronized (this) {
try {
this.wait(10000); // checked every 10s to make sure the draft moves on
} catch (InterruptedException ignore) {
}
}
if (donePicking()) {
boosterSendingEnd();
}
}
@Override
public boolean addPick(UUID playerId, UUID cardId, Set<UUID> hiddenCards) {
DraftPlayer player = players.get(playerId);
@ -354,13 +381,10 @@ public abstract class DraftImpl implements Draft {
for (Card card : player.booster) {
if (card.getId().equals(cardId)) {
player.addPick(card, hiddenCards);
player.booster.remove(card);
break;
}
}
synchronized (this) {
this.notifyAll();
}
picksCheckDone();
}
return !player.isPicking();
}

View file

@ -21,7 +21,7 @@ public class DraftPlayer {
protected Deck deck;
protected List<Card> booster;
protected boolean picking;
protected boolean boosterLoaded;
protected boolean boosterLoaded; // client confirmed that it got a booster data (for computer must be always false)
protected boolean joined = false;
protected Set<UUID> hiddenCards;
@ -64,14 +64,13 @@ public class DraftPlayer {
if (hiddenCards != null) {
this.hiddenCards = hiddenCards;
}
synchronized (booster) {
booster.remove(card);
}
picking = false;
}
public void setBooster(List<Card> booster) {
public void setBoosterAndLoad(List<Card> booster) {
this.booster = booster;
this.boosterLoaded = false; // human will receive new pick, computer with choose new pick
}
public List<Card> getBooster() {
@ -83,8 +82,9 @@ public class DraftPlayer {
}
}
public void setPicking() {
picking = true;
public void setPickingAndSending() {
this.picking = true;
this.boosterLoaded = false;
}
public boolean isPicking() {

View file

@ -28,7 +28,7 @@ public class RandomBoosterDraft extends BoosterDraft {
protected void openBooster() {
if (boosterNum <= numberBoosters) {
for (DraftPlayer player: players.values()) {
player.setBooster(getNextBooster().create15CardBooster());
player.setBoosterAndLoad(getNextBooster().create15CardBooster());
}
}
}

View file

@ -20,7 +20,7 @@ public class ReshuffledBoosterDraft extends BoosterDraft {
protected void openBooster() {
if (boosterNum <= numberBoosters) {
for (DraftPlayer player: players.values()) {
player.setBooster(reshuffledSet.createBooster());
player.setBoosterAndLoad(reshuffledSet.createBooster());
}
}
}

View file

@ -1,15 +1,14 @@
package mage.game.draft;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
import mage.cards.Card;
import mage.cards.ExpansionSet;
import org.apache.log4j.Logger;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
/**
*
* @author spjspj
*/
public class RichManBoosterDraft extends DraftImpl {
@ -38,6 +37,7 @@ public class RichManBoosterDraft extends DraftImpl {
}
boosterNum++;
}
this.boosterSendingEnd();
this.fireEndDraftEvent();
}
@ -50,7 +50,7 @@ public class RichManBoosterDraft extends DraftImpl {
DraftPlayer next = players.get(nextId);
while (true) {
List<Card> nextBooster = sets.get((cardNum - 1) % sets.size()).createBooster();
next.setBooster(nextBooster);
next.setBoosterAndLoad(nextBooster);
if (Objects.equals(nextId, startId)) {
break;
}
@ -62,22 +62,21 @@ public class RichManBoosterDraft extends DraftImpl {
@Override
protected boolean pickCards() {
synchronized (players) {
for (DraftPlayer player : players.values()) {
if (cardNum > 36) {
return false;
}
player.setPicking();
player.getPlayer().pickCard(player.getBooster(), player.getDeck(), this);
player.setPickingAndSending();
}
cardNum++;
synchronized (this) {
}
while (!donePicking()) {
try {
this.wait();
} catch (InterruptedException ex) {
}
}
boosterSendingStart();
picksWait();
}
cardNum++;
return true;
}
@ -85,6 +84,7 @@ public class RichManBoosterDraft extends DraftImpl {
public void firePickCardEvent(UUID playerId) {
DraftPlayer player = players.get(playerId);
int cardNum = Math.min(36, this.cardNum);
// richman uses custom times
int time = (int) Math.ceil(customProfiTimes[cardNum - 1] * timing.getCustomTimeoutFactor());
playerQueryEventSource.pickCard(playerId, "Pick card", player.getBooster(), time);

View file

@ -1,13 +1,12 @@
package mage.game.draft;
import java.util.*;
import mage.cards.Card;
import mage.cards.ExpansionSet;
import mage.game.draft.DraftCube.CardIdentity;
import java.util.*;
/**
*
* @author spjspj
*/
public class RichManCubeBoosterDraft extends DraftImpl {
@ -36,6 +35,7 @@ public class RichManCubeBoosterDraft extends DraftImpl {
}
boosterNum++;
}
this.boosterSendingEnd();
this.fireEndDraftEvent();
}
@ -65,7 +65,7 @@ public class RichManCubeBoosterDraft extends DraftImpl {
}
List<Card> nextBooster = draftCube.createBooster();
next.setBooster(nextBooster);
next.setBoosterAndLoad(nextBooster);
if (Objects.equals(nextId, startId)) {
break;
}
@ -77,22 +77,21 @@ public class RichManCubeBoosterDraft extends DraftImpl {
@Override
protected boolean pickCards() {
synchronized (players) {
for (DraftPlayer player : players.values()) {
if (cardNum > 36) {
return false;
}
player.setPicking();
player.getPlayer().pickCard(player.getBooster(), player.getDeck(), this);
player.setPickingAndSending();
}
cardNum++;
synchronized (this) {
}
while (!donePicking()) {
try {
this.wait();
} catch (InterruptedException ex) {
}
}
boosterSendingStart();
picksWait();
}
cardNum++;
return true;
}

View file

@ -336,7 +336,7 @@ public abstract class MatchImpl implements Match {
while (!isDoneSideboarding()) {
try {
this.wait();
} catch (InterruptedException ex) {
} catch (InterruptedException ignore) {
}
}
}