FIXED - bug 283204: [DataBinding] CheckableCheckedElementsObservableSet#clear can cause ConcurrentModificationException
https://bugs.eclipse.org/bugs/show_bug.cgi?id=283204
diff --git a/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsObservableSet.java b/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsObservableSet.java
index 1b77a97..09a29fa 100644
--- a/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsObservableSet.java
+++ b/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsObservableSet.java
@@ -7,14 +7,13 @@
*
* Contributors:
* Matthew Hall - initial API and implementation (bug 124684)
- * Matthew Hall - bug 259380
+ * Matthew Hall - bug 259380, 283204
******************************************************************************/
package org.eclipse.jface.internal.databinding.viewers;
import java.util.Collection;
import java.util.Collections;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
@@ -25,6 +24,7 @@
import org.eclipse.jface.viewers.CheckStateChangedEvent;
import org.eclipse.jface.viewers.ICheckStateListener;
import org.eclipse.jface.viewers.ICheckable;
+import org.eclipse.jface.viewers.IElementComparer;
/**
*
@@ -35,6 +35,7 @@
private ICheckable checkable;
private Set wrappedSet;
private Object elementType;
+ private IElementComparer elementComparer;
private ICheckStateListener listener;
/**
@@ -46,17 +47,21 @@
* the set being wrapped
* @param elementType
* type of elements in the set
+ * @param elementComparer
+ * element comparer
* @param checkable
* the ICheckable to track
*/
public CheckableCheckedElementsObservableSet(Realm realm,
- final Set wrappedSet, Object elementType, ICheckable checkable) {
+ final Set wrappedSet, Object elementType,
+ IElementComparer elementComparer, ICheckable checkable) {
super(realm);
Assert.isNotNull(checkable, "Checkable cannot be null"); //$NON-NLS-1$
Assert.isNotNull(wrappedSet, "Wrapped set cannot be null"); //$NON-NLS-1$
this.checkable = checkable;
this.wrappedSet = wrappedSet;
this.elementType = elementType;
+ this.elementComparer = elementComparer;
listener = new ICheckStateListener() {
public void checkStateChanged(CheckStateChangedEvent event) {
@@ -81,7 +86,7 @@
}
Set createDiffSet() {
- return new HashSet();
+ return ViewerElementSet.withComparer(elementComparer);
}
public Object getElementType() {
@@ -165,7 +170,10 @@
}
public void clear() {
- removeAll(wrappedSet);
+ getterCalled();
+ Set removals = createDiffSet();
+ removals.addAll(wrappedSet);
+ removeAll(removals);
}
public Iterator iterator() {
diff --git a/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsProperty.java b/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsProperty.java
index 541e0b2..78a6688 100644
--- a/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsProperty.java
+++ b/bundles/org.eclipse.jface.databinding/src/org/eclipse/jface/internal/databinding/viewers/CheckableCheckedElementsProperty.java
@@ -7,11 +7,11 @@
*
* Contributors:
* Matthew Hall - initial API and implementation (bug 259380)
+ * Matthew Hall - bug 283204
******************************************************************************/
package org.eclipse.jface.internal.databinding.viewers;
-import java.util.HashSet;
import java.util.Set;
import org.eclipse.core.databinding.observable.Realm;
@@ -19,6 +19,7 @@
import org.eclipse.core.databinding.property.set.SetProperty;
import org.eclipse.jface.databinding.swt.SWTObservables;
import org.eclipse.jface.viewers.ICheckable;
+import org.eclipse.jface.viewers.IElementComparer;
import org.eclipse.jface.viewers.StructuredViewer;
import org.eclipse.jface.viewers.Viewer;
@@ -49,12 +50,12 @@
}
public IObservableSet observe(Realm realm, Object source) {
- Set wrappedSet = new HashSet();
+ IElementComparer comparer = null;
if (source instanceof StructuredViewer)
- wrappedSet = ViewerElementSet
- .withComparer(((StructuredViewer) source).getComparer());
+ comparer = ((StructuredViewer) source).getComparer();
+ Set wrappedSet = ViewerElementSet.withComparer(comparer);
IObservableSet observable = new CheckableCheckedElementsObservableSet(
- realm, wrappedSet, elementType, (ICheckable) source);
+ realm, wrappedSet, elementType, comparer, (ICheckable) source);
if (source instanceof Viewer)
observable = new ViewerObservableSetDecorator(observable,
(Viewer) source);
diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/BindingTestSuite.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/BindingTestSuite.java
index 17c6703..6e5dd72 100644
--- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/BindingTestSuite.java
+++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/databinding/BindingTestSuite.java
@@ -15,7 +15,8 @@
* 212518, 215531, 221351, 184830, 213145, 218269, 239015,
* 237703, 237718, 222289, 247394, 233306, 247647, 254524,
* 246103, 249992, 256150, 256543, 262269, 175735, 262946,
- * 255734, 263693, 169876, 266038, 268336, 270461, 271720
+ * 255734, 263693, 169876, 266038, 268336, 270461, 271720,
+ * 283204
* Ovidio Mallo - bug 237163, bug 235195
*******************************************************************************/
package org.eclipse.jface.tests.databinding;
@@ -201,6 +202,7 @@
import org.eclipse.jface.tests.internal.databinding.swt.TextObservableValueFocusOutTest;
import org.eclipse.jface.tests.internal.databinding.swt.TextObservableValueModifyTest;
import org.eclipse.jface.tests.internal.databinding.swt.TextObservableValueTest;
+import org.eclipse.jface.tests.internal.databinding.viewers.CheckableCheckedElementsObservableSetTest;
import org.eclipse.jface.tests.internal.databinding.viewers.ObservableCollectionContentProviderTest;
import org.eclipse.jface.tests.internal.databinding.viewers.ObservableCollectionTreeContentProviderTest;
import org.eclipse.jface.tests.internal.databinding.viewers.ObservableViewerElementSetTest;
@@ -453,6 +455,7 @@
addTestSuite(StyledTextObservableValueTest.class);
// org.eclipse.jface.tests.internal.databinding.viewers
+ addTestSuite(CheckableCheckedElementsObservableSetTest.class);
addTest(ObservableViewerElementSetTest.suite());
addTestSuite(ObservableCollectionContentProviderTest.class);
addTestSuite(ObservableCollectionTreeContentProviderTest.class);
diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/internal/databinding/viewers/CheckableCheckedElementsObservableSetTest.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/internal/databinding/viewers/CheckableCheckedElementsObservableSetTest.java
new file mode 100644
index 0000000..d5983c8
--- /dev/null
+++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/jface/tests/internal/databinding/viewers/CheckableCheckedElementsObservableSetTest.java
@@ -0,0 +1,55 @@
+/*******************************************************************************
+ * Copyright (c) 2009 Matthew Hall and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Matthew Hall - initial API and implementation (bug 283204)
+ ******************************************************************************/
+
+package org.eclipse.jface.tests.internal.databinding.viewers;
+
+import org.eclipse.core.databinding.observable.set.IObservableSet;
+import org.eclipse.jface.databinding.viewers.ViewersObservables;
+import org.eclipse.jface.tests.databinding.AbstractDefaultRealmTestCase;
+import org.eclipse.jface.viewers.ICheckStateListener;
+import org.eclipse.jface.viewers.ICheckable;
+
+public class CheckableCheckedElementsObservableSetTest extends
+ AbstractDefaultRealmTestCase {
+ public void testClear() {
+ // init
+ ICheckable checkable = new ICheckable() {
+
+ public void addCheckStateListener(ICheckStateListener listener) {
+ }
+
+ public boolean getChecked(Object element) {
+ return false;
+ }
+
+ public void removeCheckStateListener(ICheckStateListener listener) {
+
+ }
+
+ public boolean setChecked(Object element, boolean state) {
+ return false;
+ }
+
+ };
+
+ // CheckableCheckedElementsObservableSet set = new
+ // CheckableCheckedElementsObservableSet(Realm.getDefault(),
+ // checkable, String.class);
+ IObservableSet set = ViewersObservables.observeCheckedElements(
+ checkable, String.class);
+ set.add("Test1");
+ set.add("Test2");
+ assertEquals(2, set.size());
+
+ // test
+ set.clear();
+ }
+}
