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.
