Thank you, I got it. I have one more a bit unrelated question.
The checked, synchronized and unmodifiable wrappers in some cases store
backing collection in more than one fields.
Thus `UnmodifiableList<E>` has
1. its own field `List<? extends E> list` and
2. `Collection<? extends E> c`, which it inherits from
`UnmodifiableCollection<E>`.
Also `UnmodifiableNavigableSet<E>` has even 3 duplicated fields:
1. its own `NavigableSet<E> ns`,
2. `SortedSet<E> ss` from `UnmodifiableSortedSet<E>` and
3. `Collection<? extends E> c` from `UnmodifiableCollection<E>`.
Isn't it worth to get rid of such duplication? (at least for unmodifiable
collections). This may affect serialization, but I believe it's still
possible preserve serialization backward compatible, if it's necessary.
Or is it done intentionally?
Thank you,
Dmytro
On Fri, May 1, 2020 at 1:20 AM Stuart Marks <stuart.ma...@oracle.com> wrote:
> The general rule in the collections that wrappers and views don't divulge
> their
> backing collections. The reason is fairly obvious for things like the
> checked
> wrappers and unmodifiable wrappers, but it's equally important for various
> view
> collections like those of Lists, Sets, and Maps. If there were something
> like
> getSyncRoot() it could be used to break encapsulation. For example:
>
> Map<K, V> map = Collections.synchronizedMap(...);
> someMethod(map.values());
>
> Currently, the caller is assured that someMethod() can only see the values
> of
> the map. Also, as you observe, someMethod() can't safely iterate that
> collection
> using an iterator or stream. If getSyncRoot() were introduced to address
> that issue,
>
> void someMethod(Collection<V> values) {
> @SuppressWarnings("unchecked")
> var map = (Map<K,V>) Collections.getSyncRoot(values);
> // I now have access to map's keys and can put new entries,
> mwahaha!
> }
>
> Undoubtedly there are ways we can avoid this, but the cost is designing
> yet more
> complicated APIs in an area where it provides little value. I think it's
> pretty
> unlikely that we'll do anything like this, or variants that allow the
> caller to
> provide an external lock at creation time, such as proposed in
> JDK-4335520.
> There's a pretty fundamental clash between external locking and
> encapsulation,
> and the platform has shifted to pursuing other approaches for concurrency.
>
> Legacy code that needs to iterate collection views might find some luck
> replacing iterator loops with bulk methods like forEach() or removeIf().
>
> s'marks
>
>
> On 4/30/20 1:34 AM, dmytro sheyko wrote:
> > Hi Stuart,
> >
> > In general I agree with you regarding synchronized collections. But
> nevertheless
> > there is a lot of legacy code that still uses synchronized wrappers. And
> > sometimes synchronization is done on wrong lock object, which leads to
> > relatively rare but extremely hard to reproduce and troubleshoot errors.
> > Reworking whole this legacy stuff is risky. But if we had means to get
> the right
> > lock object for given synchronized collections, this would help us to
> make the
> > fixes more localized and hence safe. That's why I would like to know the
> reason,
> > why this has not been done earlier, and is there hope/plan this will be
> done in
> > near future.
> >
> > Thank you,
> > Dmytro
> >
> > On Thu, Apr 30, 2020 at 6:36 AM Stuart Marks <stuart.ma...@oracle.com
> > <mailto:stuart.ma...@oracle.com>> wrote:
> >
> > Hi Dmytro,
> >
> > Callers of an API performing explicit synchronization, along with the
> > synchronized collections wrappers, have mostly fallen into disuse
> since the
> > introduction of the java.util.concurrent collections.
> >
> > Multiple threads can either interact directly on a concurrent
> collection, or
> > the
> > developer can provide an intermediate object (not a collection) that
> does
> > internal locking, and that exports the right set of thread-safe APIs
> to
> > callers.
> > I'm thus skeptical of the utility of enhancing these wrapper classes
> with
> > additional APIs.
> >
> > Do you have a use case that's difficult to handle any other way?
> >
> > s'marks
> >
> >
> >
> > On 4/29/20 12:58 AM, dmytro sheyko wrote:
> > > Hello,
> > >
> > > Have you ever discussed to make field mutex in synchronized
> collections
> > > accessible?
> > >
> > > Javadoc for Collections#synchronizedSortedSet suggest to iterate
> collection
> > > this way:
> > >
> > > SortedSet s = Collections.synchronizedSortedSet(new TreeSet());
> > > SortedSet s2 = s.headSet(foo);
> > > ...
> > > synchronized (s) { // Note: s, not s2!!!
> > > Iterator i = s2.iterator(); // Must be in the synchronized
> block
> > > while (i.hasNext())
> > > foo(i.next());
> > > }
> > >
> > > I.e. in order to iterate subset, we also need a reference to the
> whole set,
> > > which is not really convenient. How about to make it possible to
> write:
> > >
> > > SortedSet s2 = s.headSet(foo);
> > > ...
> > > synchronized (Collections.getSyncRoot(s2)) { // Note:
> > > Collections.getSyncRoot(s2)
> > > Iterator i = s2.iterator(); // Must be in the synchronized
> block
> > > while (i.hasNext())
> > > foo(i.next());
> > > }
> > >
> > > Also I think it would be convenient to let to provide custom sync
> root when
> > > synchronized collection is created.
> > > E.g.
> > >
> > > Object customSyncRoot = new Object();
> > > SortedSet s = Collections.synchronizedSortedSet(new TreeSet(),
> > > customSyncRoot);
> > >
> > > What do you think about this?
> > >
> > > Regards,
> > > Dmytro
> > >
> >
>
