From 896793b93db14a532540e00cd71f2c8e0db76775 Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Mon, 1 Dec 2014 17:00:26 +0100 Subject: [PATCH] * Session locking - Fixed some problems with the reentrant lock. --- .../src/main/java/mage/server/Main.java | 11 ++- .../src/main/java/mage/server/Session.java | 68 ++++++++++++++----- .../main/java/mage/server/SessionManager.java | 42 ++++++------ .../src/main/java/mage/server/User.java | 18 ++++- 4 files changed, 92 insertions(+), 47 deletions(-) diff --git a/Mage.Server/src/main/java/mage/server/Main.java b/Mage.Server/src/main/java/mage/server/Main.java index be49f9bd5d..29697af457 100644 --- a/Mage.Server/src/main/java/mage/server/Main.java +++ b/Mage.Server/src/main/java/mage/server/Main.java @@ -63,6 +63,7 @@ import java.net.InetAddress; import java.net.MalformedURLException; import java.util.HashMap; import java.util.Map; +import static mage.server.Session.getBasicCause; import org.jboss.remoting.transport.bisocket.BisocketServerInvoker; @@ -213,18 +214,16 @@ public class Main { //SessionManager.getInstance().disconnect(client.getSessionId(), DisconnectReason.Disconnected); SessionManager.getInstance().disconnect(client.getSessionId(), DisconnectReason.LostConnection); logger.info("CLIENT DISCONNECTED - " + sessionInfo, throwable); - if (logger.isDebugEnabled()) { - throwable.printStackTrace(); - } + logger.debug("Stack Trace", throwable); } else { SessionManager.getInstance().disconnect(client.getSessionId(), DisconnectReason.LostConnection); - logger.info("CONNECTION LOST - " + sessionInfo, throwable); + logger.info("LOST CONNECTION - " + sessionInfo); if (logger.isDebugEnabled()) { if (throwable == null) { - logger.debug("Lease expired"); + logger.debug("- cause: Lease expired"); } else { - throwable.printStackTrace(); + logger.debug(" - cause: " + Session.getBasicCause(throwable).toString()); } } } diff --git a/Mage.Server/src/main/java/mage/server/Session.java b/Mage.Server/src/main/java/mage/server/Session.java index 06f7e4ab74..a805028954 100644 --- a/Mage.Server/src/main/java/mage/server/Session.java +++ b/Mage.Server/src/main/java/mage/server/Session.java @@ -33,7 +33,6 @@ import java.util.LinkedList; import java.util.List; import java.util.UUID; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -213,8 +212,10 @@ public class Session { // because different threads can activate this public void userLostConnection() { + boolean lockSet = false; try { - if(lock.tryLock(5, TimeUnit.SECONDS)) { + if(lock.tryLock(500, TimeUnit.MILLISECONDS)) { + lockSet = true; User user = UserManager.getInstance().getUser(userId); if (user == null || !user.isConnected()) { return; //user was already disconnected by other thread @@ -224,46 +225,70 @@ public class Session { logger.info("OLD SESSION IGNORED - " + user.getName()); return; } - logger.info("LOST CONNECTION - " + user.getName()); + logger.info("LOST CONNECTION - " + user.getName() + " id: " + userId); UserManager.getInstance().disconnect(userId, DisconnectReason.LostConnection); } else { - logger.error("SESSION LOCK - userId " + userId); + logger.error("SESSION LOCK lost connection - userId: " + userId); } } catch (InterruptedException ex) { - logger.error("SESSION LOCK - userId " + userId, ex); + logger.error("SESSION LOCK lost connection - userId: " + userId, ex); } finally { - lock.unlock(); + if (lockSet) { + lock.unlock(); + } } } public void kill(DisconnectReason reason) { - UserManager.getInstance().removeUser(userId, reason); + boolean lockSet = false; + try { + if(lock.tryLock(500, TimeUnit.MILLISECONDS)) { + lockSet = true; + UserManager.getInstance().removeUser(userId, reason); + } else { + logger.error("SESSION LOCK - kill: userId " + userId); + } + } catch (InterruptedException ex) { + logger.error("SESSION LOCK - kill: userId " + userId, ex); + } + finally { + if (lockSet) { + lock.unlock(); + } + } + } public void fireCallback(final ClientCallback call) { + boolean lockSet = false; try { - boolean tryLock; - if (lock.tryLock(5, TimeUnit.SECONDS)) { + if (lock.tryLock(500, TimeUnit.MILLISECONDS)) { + lockSet = true; call.setMessageId(messageId++); callbackHandler.handleCallbackOneway(new Callback(call)); } else { - logger.error("CALLBACK LOCK - userId " + userId); + logger.error("SESSION LOCK callback: userId " + userId); logger.error(" - method: " + call.getMethod()); } } catch (HandleCallbackException ex) { - logger.info("CALLBACK EXCEPTION - userId " + userId, ex); - if (logger.isDebugEnabled()) { - ex.printStackTrace(); - } + User user = UserManager.getInstance().getUser(userId); + logger.warn("SESSION CALLBACK EXCEPTION - " + (user != null ? user.getName():"") + " userId " + userId); + logger.warn(" - method: " + call.getMethod()); + logger.warn(" - cause: " + getBasicCause(ex).toString()); + logger.trace("Stack trace:", ex); userLostConnection(); } catch (InterruptedException ex) { - logger.error("CALLBACK LOCK EXCEPTION - userId " + userId, ex); + logger.error("SESSION LOCK callback -userId: " + userId); logger.error(" - method: " + call.getMethod()); + logger.error(" - cause: " + getBasicCause(ex).toString()); + logger.trace("Stack trace:", ex); } finally { - lock.unlock(); + if (lockSet) { + lock.unlock(); + } } } @@ -293,4 +318,15 @@ public class Session { messageData.add(message); fireCallback(new ClientCallback("showUserMessage", null, messageData)); } + + public static Throwable getBasicCause(Throwable cause) { + Throwable t = cause; + while (t.getCause() != null) { + t = t.getCause(); + if (t == cause) { + throw new IllegalArgumentException("Infinite cycle detected in causal chain"); + } + } + return t; + } } diff --git a/Mage.Server/src/main/java/mage/server/SessionManager.java b/Mage.Server/src/main/java/mage/server/SessionManager.java index 8615fb063d..e33d6994f9 100644 --- a/Mage.Server/src/main/java/mage/server/SessionManager.java +++ b/Mage.Server/src/main/java/mage/server/SessionManager.java @@ -115,28 +115,26 @@ public class SessionManager { Session session = sessions.get(sessionId); if (session != null) { if (!reason.equals(DisconnectReason.AdminDisconnect)) { - synchronized (session) { - if (!sessions.containsKey(sessionId)) { - // session was removed meanwhile by another thread so we can return - return; - } - sessions.remove(sessionId); - switch (reason) { - case Disconnected: - session.kill(reason); - LogServiceImpl.instance.log(LogKeys.KEY_SESSION_KILLED, sessionId); - break; - case SessionExpired: - session.kill(reason); - LogServiceImpl.instance.log(LogKeys.KEY_SESSION_EXPIRED, sessionId); - break; - case LostConnection: - session.userLostConnection(); - LogServiceImpl.instance.log(LogKeys.KEY_SESSION_DISCONNECTED, sessionId); - break; - default: - logger.error("endSession: unexpected reason " + reason.toString() + " - sessionId: "+ sessionId); - } + if (!sessions.containsKey(sessionId)) { + // session was removed meanwhile by another thread so we can return + return; + } + sessions.remove(sessionId); + switch (reason) { + case Disconnected: + session.kill(reason); + LogServiceImpl.instance.log(LogKeys.KEY_SESSION_KILLED, sessionId); + break; + case SessionExpired: + session.kill(reason); + LogServiceImpl.instance.log(LogKeys.KEY_SESSION_EXPIRED, sessionId); + break; + case LostConnection: + session.userLostConnection(); + LogServiceImpl.instance.log(LogKeys.KEY_SESSION_DISCONNECTED, sessionId); + break; + default: + logger.error("endSession: unexpected reason " + reason.toString() + " - sessionId: "+ sessionId); } } else { sessions.remove(sessionId); diff --git a/Mage.Server/src/main/java/mage/server/User.java b/Mage.Server/src/main/java/mage/server/User.java index c13ebdff79..a4f501884d 100644 --- a/Mage.Server/src/main/java/mage/server/User.java +++ b/Mage.Server/src/main/java/mage/server/User.java @@ -29,6 +29,7 @@ package mage.server; import java.util.ArrayList; import java.util.Date; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -123,14 +124,25 @@ public class User { this.sessionId = sessionId; if (sessionId.isEmpty()) { userState = UserState.Disconnected; - logger.debug("Disconnected User " + userName + " id: " + userId); + lostConnection(); + logger.debug("USER - lost connection: " + userName + " id: " + userId); + } else if (userState == UserState.Created) { userState = UserState.Connected; - logger.debug("Created user " + userName + " id: " + userId); + logger.debug("USER - created: " + userName + " id: " + userId); } else { userState = UserState.Reconnected; reconnect(); - logger.info("Reconnected user " + userName + " id: " + userId); + logger.info("USER - reconnected: " + userName + " id: " + userId); + } + } + + public void lostConnection() { + // Because watched games don't get restored after reconnection call stop watching + for (Iterator iterator = watchedGames.iterator(); iterator.hasNext();) { + UUID gameId = iterator.next(); + GameManager.getInstance().stopWatching(gameId, userId); + iterator.remove(); } }