From 8547f9270a761363d6dd672e7a6104fb1a562eea Mon Sep 17 00:00:00 2001
From: pkupczyk <piotr.kupczyk@id.ethz.ch>
Date: Wed, 6 Dec 2023 17:33:56 +0100
Subject: [PATCH] SSDM-14210 : Deadlock between DefaultSessionManager and
PATSessionManagerDecorator
---
.../authentication/DefaultSessionManager.java | 16 ++++++++--------
.../cisd/authentication/SessionManagerLock.java | 13 +++++++++++++
.../generic/server/OpenBisSessionManager.java | 3 ++-
...ccessTokenOpenBisSessionManagerDecorator.java | 13 +++++++------
4 files changed, 30 insertions(+), 15 deletions(-)
create mode 100644 lib-authentication/source/java/ch/systemsx/cisd/authentication/SessionManagerLock.java
diff --git a/lib-authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java b/lib-authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java
index 8005237900d..db7cac6b7eb 100644
--- a/lib-authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java
+++ b/lib-authentication/source/java/ch/systemsx/cisd/authentication/DefaultSessionManager.java
@@ -214,7 +214,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
{
final String sessionToken = SessionTokenHash.create(user, now).toString();
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
int maxNumberOfSessions = getMaxNumberOfSessionsFor(user);
if (maxNumberOfSessions > 0)
@@ -269,7 +269,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
{
if (sessionMonitor == null)
{
- synchronized (this)
+ synchronized (SessionManagerLock.getInstance())
{
if (sessionMonitor == null)
{
@@ -395,7 +395,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
public boolean isSessionActive(final String sessionToken)
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
final FullSession<T> session = sessions.get(sessionToken);
if (session != null)
@@ -508,7 +508,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
@Override public List<T> getSessions()
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
List<T> result = new ArrayList<>();
for (FullSession<T> session : sessions.values())
@@ -522,7 +522,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
@Override
public T tryGetSession(String sessionToken)
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
final FullSession<T> session = sessions.get(sessionToken);
return (session == null) ? null : session.getSession();
@@ -534,7 +534,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
{
checkIfNotBlank(sessionToken, "sessionToken");
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
final FullSession<T> session = sessions.get(sessionToken);
@@ -633,7 +633,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
@Override
public void closeSession(final String sessionToken) throws InvalidSessionException
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
final T session = getSession(sessionToken, false);
closeSession(session, SessionClosingReason.LOGOUT);
@@ -650,7 +650,7 @@ public class DefaultSessionManager<T extends BasicSession> implements ISessionMa
private void closeSession(final T session, SessionClosingReason reason)
throws InvalidSessionException
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
session.cleanup();
String sessionToken = session.getSessionToken();
diff --git a/lib-authentication/source/java/ch/systemsx/cisd/authentication/SessionManagerLock.java b/lib-authentication/source/java/ch/systemsx/cisd/authentication/SessionManagerLock.java
new file mode 100644
index 00000000000..2dc0c3f9c8d
--- /dev/null
+++ b/lib-authentication/source/java/ch/systemsx/cisd/authentication/SessionManagerLock.java
@@ -0,0 +1,13 @@
+package ch.systemsx.cisd.authentication;
+
+public class SessionManagerLock
+{
+
+ private static final SessionManagerLock instance = new SessionManagerLock();
+
+ private SessionManagerLock(){}
+
+ public static SessionManagerLock getInstance(){
+ return instance;
+ }
+}
diff --git a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/OpenBisSessionManager.java b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/OpenBisSessionManager.java
index 1f7bb137f66..bfc7300d1d8 100644
--- a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/OpenBisSessionManager.java
+++ b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/OpenBisSessionManager.java
@@ -25,6 +25,7 @@ import ch.systemsx.cisd.authentication.DefaultSessionManager;
import ch.systemsx.cisd.authentication.IAuthenticationService;
import ch.systemsx.cisd.authentication.ILogMessagePrefixGenerator;
import ch.systemsx.cisd.authentication.ISessionFactory;
+import ch.systemsx.cisd.authentication.SessionManagerLock;
import ch.systemsx.cisd.common.server.IRemoteHostProvider;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
import ch.systemsx.cisd.openbis.generic.shared.IOpenBisSessionManager;
@@ -111,7 +112,7 @@ public class OpenBisSessionManager extends DefaultSessionManager<Session> implem
@Override
public void updateAllSessions()
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
List<Session> list = sessions.values().stream().map(FullSession::getSession).collect(Collectors.toList());
updateSessions(daoFactory, list);
diff --git a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/pat/PersonalAccessTokenOpenBisSessionManagerDecorator.java b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/pat/PersonalAccessTokenOpenBisSessionManagerDecorator.java
index 1388917db21..9a76f24c8e8 100644
--- a/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/pat/PersonalAccessTokenOpenBisSessionManagerDecorator.java
+++ b/server-application-server/source/java/ch/systemsx/cisd/openbis/generic/server/pat/PersonalAccessTokenOpenBisSessionManagerDecorator.java
@@ -33,6 +33,7 @@ import ch.systemsx.cisd.authentication.IPrincipalProvider;
import ch.systemsx.cisd.authentication.ISessionActionListener;
import ch.systemsx.cisd.authentication.ISessionFactory;
import ch.systemsx.cisd.authentication.Principal;
+import ch.systemsx.cisd.authentication.SessionManagerLock;
import ch.systemsx.cisd.common.exceptions.InvalidSessionException;
import ch.systemsx.cisd.common.logging.LogCategory;
import ch.systemsx.cisd.common.logging.LogFactory;
@@ -111,7 +112,7 @@ public class PersonalAccessTokenOpenBisSessionManagerDecorator implements IOpenB
{
sessionManager.updateAllSessions();
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
OpenBisSessionManager.updateSessions(daoFactory, sessions.values());
}
@@ -152,7 +153,7 @@ public class PersonalAccessTokenOpenBisSessionManagerDecorator implements IOpenB
return;
}
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
Session session = sessions.get(sessionToken);
@@ -181,7 +182,7 @@ public class PersonalAccessTokenOpenBisSessionManagerDecorator implements IOpenB
return;
}
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
Session session = sessions.get(sessionToken);
@@ -216,7 +217,7 @@ public class PersonalAccessTokenOpenBisSessionManagerDecorator implements IOpenB
return sessionManager.getSession(sessionToken);
} else
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
Session session = getOrCreateSessionForPATSession(patSession);
@@ -253,7 +254,7 @@ public class PersonalAccessTokenOpenBisSessionManagerDecorator implements IOpenB
return sessionManager.tryGetSession(sessionToken);
} else
{
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
return getOrCreateSessionForPATSession(patSession);
}
@@ -275,7 +276,7 @@ public class PersonalAccessTokenOpenBisSessionManagerDecorator implements IOpenB
allSessions.put(session.getSessionToken(), session);
}
- synchronized (sessions)
+ synchronized (SessionManagerLock.getInstance())
{
allSessions.putAll(sessions);
--
GitLab