>
> 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.
>
>

Reply via email to