From f65eb24027560bb918b9c6a7c004a075ebf9b4ab Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Tue, 28 Apr 2020 13:06:03 +0200
Subject: [PATCH] OO-4635: synchronize fifo map which old user windows

---
 src/main/java/org/olat/core/gui/Windows.java  | 37 +++++++-------
 src/main/java/org/olat/core/util/FIFOMap.java | 50 ++++++++-----------
 2 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/src/main/java/org/olat/core/gui/Windows.java b/src/main/java/org/olat/core/gui/Windows.java
index 39806287785..dbba15692d8 100644
--- a/src/main/java/org/olat/core/gui/Windows.java
+++ b/src/main/java/org/olat/core/gui/Windows.java
@@ -98,14 +98,14 @@ public class Windows implements Disposable, Serializable {
 		String winCmpId = ureq.getWindowComponentID();
 		
 		boolean canBeRemoved = false;
-		for(Iterator<Map.Entry<UriPrefixIdPair,ChiefController>> chiefIt=windows.getEntryIterator(); chiefIt.hasNext(); ) {
-			Map.Entry<UriPrefixIdPair,ChiefController> entry = chiefIt.next();
+		Map<UriPrefixIdPair,ChiefController> entries = windows.copyEntries();
+		for(Map.Entry<UriPrefixIdPair,ChiefController> entry:entries.entrySet()) {
 			Window window = entry.getValue().getWindow();
 			if(window.getInstanceId().equals(winId) || window.getDispatchID().equals(winCmpId)) {
 				window.setMarkToBeRemoved(false);
 			} else if(window.canBeRemoved()) {
 				window.getWindowBackOffice().dispose();
-				chiefIt.remove();
+				windows.remove(entry.getKey());
 				canBeRemoved = true;
 			}
 		}
@@ -121,8 +121,8 @@ public class Windows implements Disposable, Serializable {
 	}
 	
 	public Window getFirstWindow() {
-		for(Iterator<Map.Entry<UriPrefixIdPair,ChiefController>> chiefIt=windows.getEntryIterator(); chiefIt.hasNext(); ) {
-			Map.Entry<UriPrefixIdPair,ChiefController> entry = chiefIt.next();
+		Map<UriPrefixIdPair,ChiefController> entries = windows.copyEntries();
+		for(Map.Entry<UriPrefixIdPair,ChiefController> entry:entries.entrySet()) {
 			if("/auth/".equals(entry.getKey().uriPrefix)) {
 				return entry.getValue().getWindow();
 			}
@@ -149,8 +149,8 @@ public class Windows implements Disposable, Serializable {
 		}
 		
 		if(window == null && StringHelper.containsNonWhitespace(windowComponentId)) {
-			for(Iterator<ChiefController> chiefIt=windows.getValueIterator(); chiefIt.hasNext(); ) {
-				ChiefController cc = chiefIt.next();
+			List<ChiefController> chiefControllers = windows.values();
+			for(ChiefController cc:chiefControllers) {
 				if(windowComponentId.equals(cc.getWindow().getDispatchID())) {
 					window = cc.getWindow();
 					break;
@@ -159,8 +159,9 @@ public class Windows implements Disposable, Serializable {
 		}
 		
 		if(window == null && StringHelper.containsNonWhitespace(componentId)) {
-			for(Iterator<ChiefController> chiefIt=windows.getValueIterator(); chiefIt.hasNext(); ) {
-				Window w = chiefIt.next().getWindow();
+			List<ChiefController> chiefControllers = windows.values();
+			for(ChiefController cc:chiefControllers) {
+				Window w = cc.getWindow();
 				List<Component> cmps = new ArrayList<>();
 				Component cmp = ComponentHelper.findDescendantOrSelfByID(w.getContentPane(), componentId, cmps);
 				if(cmp != null) {
@@ -184,8 +185,8 @@ public class Windows implements Disposable, Serializable {
 		}
 		
 		if(chief == null && StringHelper.containsNonWhitespace(windowComponentId)) {
-			for(Iterator<ChiefController> chiefIt=windows.getValueIterator(); chiefIt.hasNext(); ) {
-				ChiefController cc = chiefIt.next();
+			List<ChiefController> chiefControllers = windows.values();
+			for(ChiefController cc:chiefControllers) {
 				if(windowComponentId.equals(cc.getWindow().getDispatchID())) {
 					chief = cc;
 					break;
@@ -194,8 +195,8 @@ public class Windows implements Disposable, Serializable {
 		}
 		
 		if(chief == null && StringHelper.containsNonWhitespace(componentId)) {
-			for(Iterator<ChiefController> chiefIt=windows.getValueIterator(); chiefIt.hasNext(); ) {
-				ChiefController cc = chiefIt.next();
+			List<ChiefController> chiefControllers = windows.values();
+			for(ChiefController cc:chiefControllers) {
 				List<Component> path = new ArrayList<>();
 				Component cmp = ComponentHelper.findDescendantOrSelfByID(cc.getWindow().getContentPane(), componentId, path);
 				if(cmp != null) {
@@ -264,8 +265,9 @@ public class Windows implements Disposable, Serializable {
 	 */
 	public Iterator<Window> getWindowIterator() {
 		List<Window> ws = new ArrayList<>(windows.size());
-		for(Iterator<ChiefController> chiefIt=windows.getValueIterator(); chiefIt.hasNext(); ) {
-			ws.add(chiefIt.next().getWindow());
+		List<ChiefController> chiefControllers = windows.values();
+		for(ChiefController chiefController:chiefControllers) {
+			ws.add(chiefController.getWindow());
 		}
 		return ws.iterator();
 	}
@@ -298,8 +300,9 @@ public class Windows implements Disposable, Serializable {
 
 	@Override
 	public void dispose() {
-		for(Iterator<ChiefController> chiefIt=windows.getValueIterator(); chiefIt.hasNext(); ) {
-			chiefIt.next().getWindow().getWindowBackOffice().dispose();
+		List<ChiefController> chiefControllers = windows.values();
+		for(ChiefController chiefController:chiefControllers) {
+			chiefController.getWindow().getWindowBackOffice().dispose();
 		}
 		windows.clear();
 	}
diff --git a/src/main/java/org/olat/core/util/FIFOMap.java b/src/main/java/org/olat/core/util/FIFOMap.java
index 317efc74e8e..e93696faacc 100644
--- a/src/main/java/org/olat/core/util/FIFOMap.java
+++ b/src/main/java/org/olat/core/util/FIFOMap.java
@@ -26,10 +26,11 @@
 
 package org.olat.core.util;
 
+import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /**
  * Description: <br>
@@ -50,16 +51,15 @@ public class FIFOMap<T,U> {
 	public FIFOMap(int maxsize) {
 		this.maxsize = maxsize;
 		lhm = new LinkedHashMap<>();
-		//Map m = lhm;
 	}
 
 	/**
-	 * put a (key,value) tupel
+	 * Put a (key,value) tuple
 	 * 
-	 * @param key
-	 * @param value
+	 * @param key The key
+	 * @param value The value
 	 */
-	public void put(T key, U value) {
+	public synchronized void put(T key, U value) {
 		lhm.put(key, value);
 		if (lhm.size() > maxsize) {
 			// removed oldest = 1. in queue
@@ -74,7 +74,7 @@ public class FIFOMap<T,U> {
 	 * @param key
 	 * @return the value for the supplied key
 	 */
-	public U get(T key) {
+	public synchronized U get(T key) {
 		return lhm.get(key);
 	}
 
@@ -83,48 +83,42 @@ public class FIFOMap<T,U> {
 	 * @param key
 	 * @return value of removed key
 	 */
-	public U remove(T key) {
+	public synchronized U remove(T key) {
 		U o = lhm.get(key);
 		lhm.remove(key);
 		return o;
 	}
 
-	/**
-	 * @see java.lang.Object#toString()
-	 */
-	@Override
-	public String toString() {
-		return "FIFOMap:" + lhm.size() + ": " + lhm.keySet().toString() + ", super:" + super.toString();
-	}
-
 	/**
 	 * @return size of this map
 	 */
-	public int size() {
+	public synchronized int size() {
 		return lhm.size();
 	}
-
+	
 	/**
-	 * @return ordered set of keys
+	 * 
+	 * @return List of values
 	 */
-	public Set<T> getOrderedKeySet() {
-		return lhm.keySet();
+	public synchronized List<U> values() {
+		return new ArrayList<>(lhm.values());
 	}
 
 	/**
-	 * @return value iterator
+	 * @return A copy of the entries
 	 */
-	public Iterator<U> getValueIterator() {
-		return lhm.values().iterator();
-	}
-	
-	public Iterator<Map.Entry<T,U>> getEntryIterator() {
-		return lhm.entrySet().iterator();
+	public synchronized Map<T,U> copyEntries() {
+		return new LinkedHashMap<>(lhm);
 	}
 	
 	public void clear() {
 		lhm.clear();
 	}
+	
+	@Override
+	public String toString() {
+		return "FIFOMap:" + lhm.size() + ": " + lhm.keySet().toString() + ", super:" + super.toString();
+	}
 
 }
 
-- 
GitLab