Skip to content

Commit 4f0f992

Browse files
author
Michael Strauß
committed
8347753: VetoableListDecorator doesn't accept its own sublists for bulk operations
Reviewed-by: angorya, kcr
1 parent f38ab33 commit 4f0f992

File tree

2 files changed

+142
-51
lines changed

2 files changed

+142
-51
lines changed

‎modules/javafx.base/src/main/java/com/sun/javafx/collections/VetoableListDecorator.java‎

Lines changed: 75 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,11 @@ public boolean setAll(E... elements) {
112112

113113
@Override
114114
public boolean setAll(Collection<? extends E> col) {
115-
Objects.requireNonNull(col);
116-
onProposedChange(Collections.unmodifiableList(new ArrayList<>(col)), 0, size());
115+
List<E> elements = unmodifiableList(col);
116+
onProposedChange(elements, 0, size());
117117
try {
118118
modCount++;
119-
return list.setAll(col);
119+
return list.setAll(elements);
120120
} catch(Exception e) {
121121
modCount--;
122122
throw e;
@@ -191,7 +191,7 @@ public boolean contains(Object o) {
191191

192192
@Override
193193
public Iterator<E> iterator() {
194-
return new VetoableIteratorDecorator(new ModCountAccessorImpl(),list.iterator(), 0);
194+
return new VetoableIteratorDecorator(this, new ModCountAccessorImpl(), list.iterator(), 0);
195195
}
196196

197197
@Override
@@ -234,11 +234,11 @@ public boolean containsAll(Collection<?> c) {
234234

235235
@Override
236236
public boolean addAll(Collection<? extends E> c) {
237-
Objects.requireNonNull(c);
238-
onProposedChange(Collections.unmodifiableList(new ArrayList<>(c)), size(), size());
237+
List<E> elements = unmodifiableList(c);
238+
onProposedChange(elements, size(), size());
239239
try {
240240
modCount++;
241-
boolean ret = list.addAll(c);
241+
boolean ret = list.addAll(elements);
242242
if (!ret)
243243
modCount--;
244244
return ret;
@@ -250,12 +250,12 @@ public boolean addAll(Collection<? extends E> c) {
250250

251251
@Override
252252
public boolean addAll(int index, Collection<? extends E> c) {
253-
Objects.requireNonNull(c);
254253
Objects.checkIndex(index, size() + 1);
255-
onProposedChange(Collections.unmodifiableList(new ArrayList<>(c)), index, index);
254+
List<E> elements = unmodifiableList(c);
255+
onProposedChange(elements, index, index);
256256
try {
257257
modCount++;
258-
boolean ret = list.addAll(index, c);
258+
boolean ret = list.addAll(index, elements);
259259
if (!ret)
260260
modCount--;
261261
return ret;
@@ -267,11 +267,11 @@ public boolean addAll(int index, Collection<? extends E> c) {
267267

268268
@Override
269269
public boolean removeAll(Collection<?> c) {
270-
Objects.requireNonNull(c);
271-
removeFromList(this, 0, c, false);
270+
Collection<?> elements = safeCollection(c);
271+
removeFromList(this, 0, elements, false);
272272
try {
273273
modCount++;
274-
boolean ret = list.removeAll(c);
274+
boolean ret = list.removeAll(elements);
275275
if (!ret)
276276
modCount--;
277277
return ret;
@@ -283,11 +283,11 @@ public boolean removeAll(Collection<?> c) {
283283

284284
@Override
285285
public boolean retainAll(Collection<?> c) {
286-
Objects.requireNonNull(c);
287-
removeFromList(this, 0, c, true);
286+
Collection<?> elements = safeCollection(c);
287+
removeFromList(this, 0, elements, true);
288288
try {
289289
modCount++;
290-
boolean ret = list.retainAll(c);
290+
boolean ret = list.retainAll(elements);
291291
if (!ret)
292292
modCount--;
293293
return ret;
@@ -359,17 +359,17 @@ public int lastIndexOf(Object o) {
359359

360360
@Override
361361
public ListIterator<E> listIterator() {
362-
return new VetoableListIteratorDecorator(new ModCountAccessorImpl(), list.listIterator(), 0);
362+
return new VetoableListIteratorDecorator(this, new ModCountAccessorImpl(), list.listIterator(), 0);
363363
}
364364

365365
@Override
366366
public ListIterator<E> listIterator(int index) {
367-
return new VetoableListIteratorDecorator(new ModCountAccessorImpl(), list.listIterator(index), index);
367+
return new VetoableListIteratorDecorator(this, new ModCountAccessorImpl(), list.listIterator(index), index);
368368
}
369369

370370
@Override
371371
public List<E> subList(int fromIndex, int toIndex) {
372-
return new VetoableSubListDecorator(new ModCountAccessorImpl(), list.subList(fromIndex, toIndex), fromIndex);
372+
return new VetoableSubListDecorator(this, new ModCountAccessorImpl(), list.subList(fromIndex, toIndex), fromIndex);
373373
}
374374

375375
@Override
@@ -387,14 +387,37 @@ public int hashCode() {
387387
return list.hashCode();
388388
}
389389

390-
private class VetoableSubListDecorator implements List<E> {
390+
/**
391+
* Returns the specified collection as an unmodifiable list that can safely be used in all bulk
392+
* operations without triggering {@link ConcurrentModificationException}.
393+
*/
394+
private <T> List<T> unmodifiableList(Collection<? extends T> c) {
395+
Objects.requireNonNull(c);
396+
return !(c instanceof List<?>) || (c instanceof VetoableSubListDecorator<?> d && d.parent == this)
397+
? Collections.unmodifiableList(new ArrayList<>(c))
398+
: Collections.unmodifiableList((List<T>)c);
399+
}
400+
401+
/**
402+
* Returns a collection that can safely be used in the {@link #removeAll(Collection)} and
403+
* {@link #retainAll(Collection)} operations without triggering {@link ConcurrentModificationException}.
404+
*/
405+
private <T> Collection<T> safeCollection(Collection<T> c) {
406+
Objects.requireNonNull(c);
407+
return c instanceof VetoableSubListDecorator<?> d && d.parent == this
408+
? (List<T>)Arrays.asList(c.toArray())
409+
: c;
410+
}
391411

412+
private static class VetoableSubListDecorator<E> implements List<E> {
413+
private final VetoableListDecorator parent;
392414
private final List<E> subList;
393415
private final int offset;
394416
private final ModCountAccessor modCountAccessor;
395417
private int modCount;
396418

397-
public VetoableSubListDecorator(ModCountAccessor modCountAccessor, List<E> subList, int offset) {
419+
public VetoableSubListDecorator(VetoableListDecorator<E> parent, ModCountAccessor modCountAccessor, List<E> subList, int offset) {
420+
this.parent = parent;
398421
this.modCountAccessor = modCountAccessor;
399422
this.modCount = modCountAccessor.get();
400423
this.subList = subList;
@@ -423,7 +446,7 @@ public boolean contains(Object o) {
423446
@Override
424447
public Iterator<E> iterator() {
425448
checkForComodification();
426-
return new VetoableIteratorDecorator(new ModCountAccessorImplSub(), subList.iterator(), offset);
449+
return new VetoableIteratorDecorator(parent, new ModCountAccessorImplSub(), subList.iterator(), offset);
427450
}
428451

429452
@Override
@@ -441,7 +464,7 @@ public <T> T[] toArray(T[] a) {
441464
@Override
442465
public boolean add(E e) {
443466
checkForComodification();
444-
onProposedChange(Collections.<E>singletonList(e), offset + size(), offset + size());
467+
parent.onProposedChange(Collections.<E>singletonList(e), offset + size(), offset + size());
445468
try {
446469
incrementModCount();
447470
subList.add(e);
@@ -471,12 +494,12 @@ public boolean containsAll(Collection<?> c) {
471494

472495
@Override
473496
public boolean addAll(Collection<? extends E> c) {
474-
Objects.requireNonNull(c);
475497
checkForComodification();
476-
onProposedChange(Collections.unmodifiableList(new ArrayList<>(c)), offset + size(), offset + size());
498+
List<E> elements = parent.unmodifiableList(c);
499+
parent.onProposedChange(elements, offset + size(), offset + size());
477500
try {
478501
incrementModCount();
479-
boolean res = subList.addAll(c);
502+
boolean res = subList.addAll(elements);
480503
if (!res)
481504
decrementModCount();
482505
return res;
@@ -488,13 +511,13 @@ public boolean addAll(Collection<? extends E> c) {
488511

489512
@Override
490513
public boolean addAll(int index, Collection<? extends E> c) {
491-
Objects.requireNonNull(c);
492514
Objects.checkIndex(index, size() + 1);
493515
checkForComodification();
494-
onProposedChange(Collections.unmodifiableList(new ArrayList<>(c)), offset + index, offset + index);
516+
List<E> elements = parent.unmodifiableList(c);
517+
parent.onProposedChange(elements, offset + index, offset + index);
495518
try {
496519
incrementModCount();
497-
boolean res = subList.addAll(index, c);
520+
boolean res = subList.addAll(index, elements);
498521
if (!res)
499522
decrementModCount();
500523
return res;
@@ -506,12 +529,12 @@ public boolean addAll(int index, Collection<? extends E> c) {
506529

507530
@Override
508531
public boolean removeAll(Collection<?> c) {
509-
Objects.requireNonNull(c);
510532
checkForComodification();
511-
removeFromList(this, offset, c, false);
533+
Collection<?> elements = parent.safeCollection(c);
534+
parent.removeFromList(this, offset, elements, false);
512535
try {
513536
incrementModCount();
514-
boolean res = subList.removeAll(c);
537+
boolean res = subList.removeAll(elements);
515538
if (!res)
516539
decrementModCount();
517540
return res;
@@ -523,12 +546,12 @@ public boolean removeAll(Collection<?> c) {
523546

524547
@Override
525548
public boolean retainAll(Collection<?> c) {
526-
Objects.requireNonNull(c);
527549
checkForComodification();
528-
removeFromList(this, offset, c, true);
550+
Collection<?> elements = parent.safeCollection(c);
551+
parent.removeFromList(this, offset, elements, true);
529552
try {
530553
incrementModCount();
531-
boolean res = subList.retainAll(c);
554+
boolean res = subList.retainAll(elements);
532555
if (!res)
533556
decrementModCount();
534557
return res;
@@ -541,7 +564,7 @@ public boolean retainAll(Collection<?> c) {
541564
@Override
542565
public void clear() {
543566
checkForComodification();
544-
onProposedChange(Collections.<E>emptyList(), offset, offset + size());
567+
parent.onProposedChange(Collections.<E>emptyList(), offset, offset + size());
545568
try {
546569
incrementModCount();
547570
subList.clear();
@@ -560,15 +583,15 @@ public E get(int index) {
560583
@Override
561584
public E set(int index, E element) {
562585
checkForComodification();
563-
onProposedChange(Collections.singletonList(element), offset + index, offset + index + 1);
586+
parent.onProposedChange(Collections.singletonList(element), offset + index, offset + index + 1);
564587
return subList.set(index, element);
565588
}
566589

567590
@Override
568591
public void add(int index, E element) {
569592
Objects.checkIndex(index, size() + 1);
570593
checkForComodification();
571-
onProposedChange(Collections.singletonList(element), offset + index, offset + index);
594+
parent.onProposedChange(Collections.singletonList(element), offset + index, offset + index);
572595
try {
573596
incrementModCount();
574597
subList.add(index, element);
@@ -582,7 +605,7 @@ public void add(int index, E element) {
582605
public E remove(int index) {
583606
Objects.checkIndex(index, size());
584607
checkForComodification();
585-
onProposedChange(Collections.<E>emptyList(), offset + index, offset + index + 1);
608+
parent.onProposedChange(Collections.<E>emptyList(), offset + index, offset + index + 1);
586609
try {
587610
incrementModCount();
588611
E res = subList.remove(index);
@@ -609,21 +632,21 @@ public int lastIndexOf(Object o) {
609632
@Override
610633
public ListIterator<E> listIterator() {
611634
checkForComodification();
612-
return new VetoableListIteratorDecorator(new ModCountAccessorImplSub(),
635+
return new VetoableListIteratorDecorator(parent, new ModCountAccessorImplSub(),
613636
subList.listIterator(), offset);
614637
}
615638

616639
@Override
617640
public ListIterator<E> listIterator(int index) {
618641
checkForComodification();
619-
return new VetoableListIteratorDecorator(new ModCountAccessorImplSub(),
642+
return new VetoableListIteratorDecorator(parent, new ModCountAccessorImplSub(),
620643
subList.listIterator(index), offset + index);
621644
}
622645

623646
@Override
624647
public List<E> subList(int fromIndex, int toIndex) {
625648
checkForComodification();
626-
return new VetoableSubListDecorator(new ModCountAccessorImplSub(),
649+
return new VetoableSubListDecorator(parent, new ModCountAccessorImplSub(),
627650
subList.subList(fromIndex, toIndex), offset + fromIndex);
628651
}
629652

@@ -678,16 +701,17 @@ public int decrementAndGet() {
678701
}
679702
}
680703

681-
private class VetoableIteratorDecorator implements Iterator<E> {
682-
704+
private static class VetoableIteratorDecorator<E> implements Iterator<E> {
705+
final VetoableListDecorator<E> parent;
683706
private final Iterator<E> it;
684707
private final ModCountAccessor modCountAccessor;
685708
private int modCount;
686709
protected final int offset;
687710
protected int cursor;
688711
protected int lastReturned;
689712

690-
public VetoableIteratorDecorator(ModCountAccessor modCountAccessor, Iterator<E> it, int offset) {
713+
public VetoableIteratorDecorator(VetoableListDecorator<E> parent, ModCountAccessor modCountAccessor, Iterator<E> it, int offset) {
714+
this.parent = parent;
691715
this.modCountAccessor = modCountAccessor;
692716
this.modCount = modCountAccessor.get();
693717
this.it = it;
@@ -714,7 +738,7 @@ public void remove() {
714738
if (lastReturned == -1) {
715739
throw new IllegalStateException();
716740
}
717-
onProposedChange(Collections.<E>emptyList(), offset + lastReturned, offset + lastReturned + 1);
741+
parent.onProposedChange(Collections.<E>emptyList(), offset + lastReturned, offset + lastReturned + 1);
718742
try {
719743
incrementModCount();
720744
it.remove();
@@ -741,12 +765,12 @@ protected void decrementModCount() {
741765
}
742766
}
743767

744-
private class VetoableListIteratorDecorator extends VetoableIteratorDecorator implements ListIterator<E> {
768+
private static class VetoableListIteratorDecorator<E> extends VetoableIteratorDecorator<E> implements ListIterator<E> {
745769

746770
private final ListIterator<E> lit;
747771

748-
public VetoableListIteratorDecorator(ModCountAccessor modCountAccessor, ListIterator<E> it, int offset) {
749-
super(modCountAccessor, it, offset);
772+
public VetoableListIteratorDecorator(VetoableListDecorator<E> parent, ModCountAccessor modCountAccessor, ListIterator<E> it, int offset) {
773+
super(parent, modCountAccessor, it, offset);
750774
this.lit = it;
751775
}
752776

@@ -782,14 +806,14 @@ public void set(E e) {
782806
if (lastReturned == -1) {
783807
throw new IllegalStateException();
784808
}
785-
onProposedChange(Collections.singletonList(e), offset + lastReturned, offset + lastReturned + 1);
809+
parent.onProposedChange(Collections.singletonList(e), offset + lastReturned, offset + lastReturned + 1);
786810
lit.set(e);
787811
}
788812

789813
@Override
790814
public void add(E e) {
791815
checkForComodification();
792-
onProposedChange(Collections.singletonList(e), offset + cursor, offset + cursor);
816+
parent.onProposedChange(Collections.singletonList(e), offset + cursor, offset + cursor);
793817
try {
794818
incrementModCount();
795819
lit.add(e);

0 commit comments

Comments
 (0)