From 7c54e8ce88ce0e0a5c3f7021c06a973808dec0dd Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Mon, 16 Dec 2013 10:49:31 +0100
Subject: [PATCH] OO-909: don't lock the user session but an internal object to
prevent deadlocks
---
src/main/java/org/olat/core/gui/Windows.java | 32 +++++------
.../java/org/olat/core/util/UserSession.java | 54 ++++++++++++++-----
.../core/util/session/UserSessionManager.java | 2 +-
3 files changed, 55 insertions(+), 33 deletions(-)
diff --git a/src/main/java/org/olat/core/gui/Windows.java b/src/main/java/org/olat/core/gui/Windows.java
index fa0853a2778..3654370528e 100644
--- a/src/main/java/org/olat/core/gui/Windows.java
+++ b/src/main/java/org/olat/core/gui/Windows.java
@@ -76,18 +76,12 @@ public class Windows implements Disposable, Serializable {
* @return the Windows for this user
*/
public static Windows getWindows(UserSession us) {
- Windows ws;
- synchronized(us) {//sync the creation of Windows object
- ws = (Windows)us.getEntry(SESSIONID_NAME_FOR_WINDOWS);
- if (ws == null) {
- ws = new Windows();
- // make window id kind of unique (only needed for better user convenience
- // when a user tries to bookmark an url and uses that browser bookmark
- // later
- //TODO: make error handling better
- //ws.windowId = (int) (System.currentTimeMillis() % 10) * 10 + 1; // must
- // at least be 1, since 0 is null
- us.putEntry(SESSIONID_NAME_FOR_WINDOWS, ws);
+ Windows ws = (Windows)us.getEntry(SESSIONID_NAME_FOR_WINDOWS);
+ if (ws == null) {
+ final Windows newWs = new Windows();
+ ws = (Windows)us.putEntryIfAbsent(SESSIONID_NAME_FOR_WINDOWS, newWs);
+ if(ws == null) {
+ ws = newWs;
}
}
return ws;
@@ -193,7 +187,7 @@ public class Windows implements Disposable, Serializable {
public WindowManager getWindowManager() {
return windowManagerImpl;
}
-
+
public void setAttribute(String key, Object value) {
attributes.put(key, value);
}
@@ -217,19 +211,19 @@ public class Windows implements Disposable, Serializable {
return sbws;
}
- private class UriPrefixIdPair {
+ private static class UriPrefixIdPair {
private final String uriPrefix;
- private final String windowId;
+ private final String wId;
- public UriPrefixIdPair(String uriPrefix, String windowId) {
+ public UriPrefixIdPair(String uriPrefix, String wId) {
this.uriPrefix = uriPrefix;
- this.windowId = windowId;
+ this.wId = wId;
}
@Override
public int hashCode() {
return (uriPrefix == null ? 364587 : uriPrefix.hashCode())
- + (windowId == null ? 9827 : windowId.hashCode());
+ + (wId == null ? 9827 : wId.hashCode());
}
@Override
@@ -240,7 +234,7 @@ public class Windows implements Disposable, Serializable {
if(obj instanceof UriPrefixIdPair) {
UriPrefixIdPair pair = (UriPrefixIdPair)obj;
return uriPrefix != null && uriPrefix.equals(pair.uriPrefix)
- && windowId != null && windowId.equals(pair.windowId);
+ && wId != null && wId.equals(pair.wId);
}
return false;
}
diff --git a/src/main/java/org/olat/core/util/UserSession.java b/src/main/java/org/olat/core/util/UserSession.java
index 2901aaa3b7f..3f99b95314d 100644
--- a/src/main/java/org/olat/core/util/UserSession.java
+++ b/src/main/java/org/olat/core/util/UserSession.java
@@ -81,6 +81,7 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
* things to put into that should not be clear when signing on (e.g. remember url for a direct jump)
*/
private transient Map<String,Object> nonClearedStore = new HashMap<String,Object>();
+ private transient Object lockStores = new Object();
private boolean authenticated = false;
private boolean savedSession = false;
private transient Preferences guiPreferences;
@@ -127,8 +128,12 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
this.savedSession = savedSession;
}
- public Map<String,Object> getStore() {
- return store;
+ public List<Object> getStoreValues() {
+ List<Object> values;
+ synchronized(lockStores) {
+ values = new ArrayList<Object>(store.values());
+ }
+ return values;
}
/**
@@ -136,7 +141,19 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
* @param o
*/
public void putEntry(String key, Object o) {
- store.put(key, o);
+ synchronized(lockStores) {
+ store.put(key, o);
+ }
+ }
+
+ public Object putEntryIfAbsent(String key, Object o) {
+ synchronized(lockStores) {
+ if (!store.containsKey(key)) {
+ return store.put(key, o);
+ } else {
+ return store.get(key);
+ }
+ }
}
/**
@@ -147,13 +164,15 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
if (key == null) {
return null;
}
- if (store.get(key) != null) {
- return store.get(key);
- }
- if (nonClearedStore.get(key) != null) {
- return nonClearedStore.get(key);
+ synchronized(lockStores) {
+ if (store.get(key) != null) {
+ return store.get(key);
+ }
+ if (nonClearedStore.get(key) != null) {
+ return nonClearedStore.get(key);
+ }
}
- else return null;
+ return null;
}
/**
@@ -161,7 +180,9 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
* @return removed entry
*/
public Object removeEntry(String key) {
- return store.remove(key);
+ synchronized(lockStores) {
+ return store.remove(key);
+ }
}
/**
@@ -174,7 +195,9 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
* @param o
*/
public void putEntryInNonClearedStore(String key, Object o) {
- nonClearedStore.put(key, o);
+ synchronized(lockStores) {
+ nonClearedStore.put(key, o);
+ }
}
/**
@@ -182,12 +205,14 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
* @return removed entry
*/
public Object removeEntryFromNonClearedStore(String key) {
- return nonClearedStore.remove(key);
+ synchronized(lockStores) {
+ return nonClearedStore.remove(key);
+ }
}
public List<String> getChats() {
if(chats == null) {
- synchronized(this) {
+ synchronized(lockStores) {
if(chats == null) {
chats = new ArrayList<String>(5);
}
@@ -362,6 +387,7 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
/**
* @see javax.servlet.http.HttpSessionBindingListener#valueBound(javax.servlet.http.HttpSessionBindingEvent)
*/
+ @Override
public void valueBound(HttpSessionBindingEvent be) {
if (log.isDebug()) {
log.debug("Opened UserSession:" + toString());
@@ -373,6 +399,7 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
*
* @see javax.servlet.http.HttpSessionBindingListener#valueUnbound(javax.servlet.http.HttpSessionBindingEvent)
*/
+ @Override
public void valueUnbound(HttpSessionBindingEvent be) {
try {
// the identity can be null if an loginscreen only session gets invalidated
@@ -400,6 +427,7 @@ public class UserSession implements HttpSessionBindingListener, GenericEventList
/**
* only for preference changed event
*/
+ @Override
public void event(Event event) {
//fxdiff FXOLAT-231: event on GUI Preferences extern changes
if("preferences.changed".equals(event.getCommand())) {
diff --git a/src/main/java/org/olat/core/util/session/UserSessionManager.java b/src/main/java/org/olat/core/util/session/UserSessionManager.java
index 49fe5b7127b..7e4e7ee8726 100644
--- a/src/main/java/org/olat/core/util/session/UserSessionManager.java
+++ b/src/main/java/org/olat/core/util/session/UserSessionManager.java
@@ -415,7 +415,7 @@ public class UserSessionManager implements GenericEventListener {
// notify all variables in the store (the values) about the disposal
// if
// Disposable
- List<Object> storeList = new ArrayList<Object>(usess.getStore().values());
+ List<Object> storeList = usess.getStoreValues();
for (Iterator<Object> it_storevals = storeList.iterator(); it_storevals.hasNext();) {
obj = it_storevals.next();
--
GitLab