> > If we want a map that is both synchronized and observable, then yes. In > any case, I don't see any value in trying to eliminate it from the API > (e.g., through deprecation --> deprecation for removal --> removal).
One could do this: protected Map<String, Object> criteriaLock = Collections.synchronizedMap(new HashMap<String, Object>()); protected ObservableMap<String, Object> criteria = FXCollections.observableMap(criteriaLock); and then: Map<String, Object> params; synchronized (criteriaLock) { // #91157: lock synchronized map to prevent ConcurrentModificationException params = new HashMap<>(getCriteria()); } i.e. use a synchronized "inner" map and make it observable. But I admit this is a bit clumsy since you can no longer synchronize on your "real" map and have to keep the inner map around for synchronizing. I filed https://bugs.openjdk.java.net/browse/JDK-8232524 Am Do., 17. Okt. 2019 um 15:51 Uhr schrieb Kevin Rushforth < kevin.rushfo...@oracle.com>: > Hi Robert, > > 1. Is the behavior of SynchronizedObservableMap a bug, where it uses a > new mutex and doesn't synchronize on itself a bug? > > I'd say yes, this is a bug. Clearly the class is meant to mimic the > corresponding Collections class with observability added. The fact that > it then doesn't synchronize in the same way, and that this difference > makes the class not thread-safe seems to defeat the entire purpose of > having a synchronized flavor of ObservableMap. So go ahead and file a bug. > > 2. Do we need SynchronizedObservableMap at all? > > If we want a map that is both synchronized and observable, then yes. In > any case, I don't see any value in trying to eliminate it from the API > (e.g., through deprecation --> deprecation for removal --> removal). > > -- Kevin > > > On 10/17/2019 5:48 AM, Robert Lichtenberger wrote: > > I've just stumbled upon a devious detail in > > javafx.collections.FXCollections.SynchronizedObservableMap<K, V>. > Although > > it almost looks like a twin of Collections.synchronizedMap it does not > > allow to protect copying or iterating over it the way > > Collections.synchronizedMap does. > > > > Example program: > > import java.util.Collections; > > import java.util.HashMap; > > import java.util.Map; > > import java.util.Random; > > import javafx.collections.FXCollections; > > > > public class SyncMap { > > public static void main(String[] args) { > > Random rnd = new Random(); > > Map<Integer, Integer> m = Collections.synchronizedMap(new > > HashMap<Integer, Integer>()); > > // Map<Integer, Integer> m = > > > FXCollections.synchronizedObservableMap(FXCollections.observableHashMap()); > > Thread t = new Thread(() -> { > > while (true) { > > m.put(rnd.nextInt(1000), rnd.nextInt()); > > } > > }); > > t.setDaemon(true); > > t.start(); > > Map<Integer, Integer> c = null; > > for (int i = 0; i < 100000; i ++) { > > synchronized(m) { > > c = new HashMap<>(m); > > } > > } > > System.out.println(c); > > } > > } > > > > Using Collections.synchronizedMap this works, because we synchronize on > m. > > If using FXCollections.synchronizedObservableMap this throws > > ConcurrentModificationException. The reason is that > > SynchronizedObservableMap(ObservableMap<K, V> map) { > > this(map, new Object()); > > } > > SynchronizedObservableMap uses a new Object as mutex instead of itself as > > seen in > > SynchronizedMap(Map<K,V> m) { > > this.m = Objects.requireNonNull(m); > > mutex = this; > > } > > > > Questions: > > * Is this considered a bug / a possible enhancement? Should there at > least > > be a warning against this possible pitfall? > > * Do we need synchronizedObservable-Stuff in FXCollections at all? This > > seems one of the cases where JavaFX contains classes/functionality that > are > > part of the JDK itself. > > > > I can file an issue, if there is consensus about the issue. > >