Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSebastian Zarnekow2020-03-16 17:12:37 +0000
committerLars Vogel2020-03-19 16:00:59 +0000
commit2619fb4e18c333733171f051e12c442d7be4dbf6 (patch)
treeed8ff910ca7e492d0339af5e724552b2abff988f
parentcd9867e8b77f49bb72b90cd863c1aa5184d9615b (diff)
downloadeclipse.platform.text-2619fb4e18c333733171f051e12c442d7be4dbf6.tar.gz
eclipse.platform.text-2619fb4e18c333733171f051e12c442d7be4dbf6.tar.xz
eclipse.platform.text-2619fb4e18c333733171f051e12c442d7be4dbf6.zip
Bug 401391 - ConcurrentModificationException in AnnotationModelI20200319-1800
Use a ConcurrentHashMap for the AnnotationModel attachments to avoid a concurrent modification when the AnnotationIterator is created Change-Id: I3ceeebea8cd7c894fe70adae2378db59c2ac1f1b Signed-off-by: Sebastian Zarnekow <sebastian.zarnekow@gmail.com>
-rw-r--r--org.eclipse.text.tests/META-INF/MANIFEST.MF2
-rw-r--r--org.eclipse.text.tests/pom.xml2
-rw-r--r--org.eclipse.text.tests/src/org/eclipse/text/tests/Bug401391Test.java165
-rw-r--r--org.eclipse.text/META-INF/MANIFEST.MF2
-rw-r--r--org.eclipse.text/src/org/eclipse/jface/text/source/AnnotationModel.java7
5 files changed, 172 insertions, 6 deletions
diff --git a/org.eclipse.text.tests/META-INF/MANIFEST.MF b/org.eclipse.text.tests/META-INF/MANIFEST.MF
index 5b946692f..69265e595 100644
--- a/org.eclipse.text.tests/META-INF/MANIFEST.MF
+++ b/org.eclipse.text.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Plugin.name
Bundle-SymbolicName: org.eclipse.text.tests
-Bundle-Version: 3.12.500.qualifier
+Bundle-Version: 3.12.600.qualifier
Bundle-Vendor: %Plugin.providerName
Bundle-Localization: plugin
Export-Package:
diff --git a/org.eclipse.text.tests/pom.xml b/org.eclipse.text.tests/pom.xml
index 456f3b626..901321daf 100644
--- a/org.eclipse.text.tests/pom.xml
+++ b/org.eclipse.text.tests/pom.xml
@@ -19,7 +19,7 @@
</parent>
<groupId>org.eclipse.text</groupId>
<artifactId>org.eclipse.text.tests</artifactId>
- <version>3.12.500-SNAPSHOT</version>
+ <version>3.12.600-SNAPSHOT</version>
<packaging>eclipse-test-plugin</packaging>
<properties>
<testSuite>${project.artifactId}</testSuite>
diff --git a/org.eclipse.text.tests/src/org/eclipse/text/tests/Bug401391Test.java b/org.eclipse.text.tests/src/org/eclipse/text/tests/Bug401391Test.java
new file mode 100644
index 000000000..942abb294
--- /dev/null
+++ b/org.eclipse.text.tests/src/org/eclipse/text/tests/Bug401391Test.java
@@ -0,0 +1,165 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Sebastian Zarnekow and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Sebastian Zarnekow - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.text.tests;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+import java.lang.reflect.Field;
+import java.util.AbstractMap;
+import java.util.AbstractSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.eclipse.jface.text.Document;
+import org.eclipse.jface.text.Position;
+import org.eclipse.jface.text.source.Annotation;
+import org.eclipse.jface.text.source.AnnotationModel;
+
+public class Bug401391Test {
+
+ private Document fDocument;
+ private AnnotationModel fAnnotationModel;
+ private AnnotationModel fFirstInnerModel;
+ private AnnotationModel fSecondInnerModel;
+
+ @Before
+ public void setUp() throws Exception {
+ fDocument = new Document("123456789");
+
+ fAnnotationModel = new AnnotationModel();
+ fAnnotationModel.addAnnotation(new Annotation(false), new Position(0, 1));
+
+ fFirstInnerModel = new AnnotationModel();
+ fFirstInnerModel.addAnnotation(new Annotation(false), new Position(1, 2));
+ fAnnotationModel.addAnnotationModel("first", fFirstInnerModel);
+
+ fAnnotationModel.connect(fDocument);
+ }
+
+ /*
+ * The test simulates a concurrent modification of the attachments by installing
+ * a trap into the annotation model that will modify the available attachments
+ * when the attachments are iterated.
+ */
+ private void installAttachmentTrap() throws Exception {
+ installTrap(fAnnotationModel, "fAttachments", () -> {
+ if (fSecondInnerModel == null) {
+ fSecondInnerModel = new AnnotationModel();
+ fSecondInnerModel.addAnnotation(new Annotation(false), new Position(3, 2));
+ fAnnotationModel.addAnnotationModel("second", fSecondInnerModel);
+ } else {
+ fAnnotationModel.removeAnnotationModel("second");
+ }
+ });
+ }
+
+ private static void installTrap(Object target, String fieldName, Runnable onKeySetIteration) throws Exception {
+ Field fld = target.getClass().getDeclaredField(fieldName);
+ fld.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Map<Object, Object> original = (Map<Object, Object>) fld.get(target);
+ class TrapMap extends AbstractMap<Object, Object> {
+ @Override
+ public Set<Object> keySet() {
+ Set<Object> delegate = original.keySet();
+
+ return new AbstractSet<Object>() {
+
+ @Override
+ public Iterator<Object> iterator() {
+ Iterator<Object> result = delegate.iterator();
+ onKeySetIteration.run();
+ return result;
+ }
+
+ @Override
+ public int size() {
+ return delegate.size();
+ }
+ };
+ }
+
+ @Override
+ public Set<Entry<Object, Object>> entrySet() {
+ return original.entrySet();
+ }
+
+ @Override
+ public Object put(Object key, Object value) {
+ return original.put(key, value);
+ }
+ }
+ fld.set(target, new TrapMap());
+ }
+
+ @After
+ public void tearDown() {
+ fAnnotationModel.disconnect(fDocument);
+ }
+
+ @Test
+ public void testNoConcurrentModificationOnAddAttachment() throws Exception {
+ installAttachmentTrap();
+ assertNull(fAnnotationModel.getAnnotationModel("second"));
+ Iterator<Annotation> iter = fAnnotationModel.getAnnotationIterator();
+ assertNotNull(fAnnotationModel.getAnnotationModel("second"));
+ assertEquals(3, count(iter));
+ }
+
+ @Test
+ public void testNoConcurrentModificationOnRemoveAttachment() throws Exception {
+ installAttachmentTrap();
+ assertNull(fAnnotationModel.getAnnotationModel("second"));
+ fAnnotationModel.getAnnotationIterator();
+ assertNotNull(fAnnotationModel.getAnnotationModel("second"));
+ Iterator<Annotation> iter = fAnnotationModel.getAnnotationIterator();
+ assertNull(fAnnotationModel.getAnnotationModel("second"));
+ assertEquals(2, count(iter));
+ }
+
+ @Test
+ public void testRemoveAnnotationWhileIterating() throws Exception {
+ Annotation removeWhileIterating = new Annotation(false);
+ fFirstInnerModel.addAnnotation(removeWhileIterating, new Position(5, 2));
+ Iterator<Annotation> iter = fAnnotationModel.getAnnotationIterator();
+ assertTrue(iter.hasNext());
+ iter.next();
+ assertTrue(iter.hasNext());
+ iter.next();
+ fFirstInnerModel.removeAnnotation(removeWhileIterating);
+ assertEquals(2, count(fAnnotationModel.getAnnotationIterator()));
+ assertTrue(iter.hasNext());
+ // Traverses a copy of the model at the time the iterator was created
+ assertSame(removeWhileIterating, iter.next());
+ }
+
+ private int count(Iterator<?> iter) {
+ int result = 0;
+ while (iter.hasNext()) {
+ result++;
+ iter.next();
+ }
+ return result;
+ }
+
+}
diff --git a/org.eclipse.text/META-INF/MANIFEST.MF b/org.eclipse.text/META-INF/MANIFEST.MF
index a7adeb868..1a693ade3 100644
--- a/org.eclipse.text/META-INF/MANIFEST.MF
+++ b/org.eclipse.text/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.text
-Bundle-Version: 3.10.100.qualifier
+Bundle-Version: 3.10.200.qualifier
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Export-Package:
diff --git a/org.eclipse.text/src/org/eclipse/jface/text/source/AnnotationModel.java b/org.eclipse.text/src/org/eclipse/jface/text/source/AnnotationModel.java
index a12f19356..d32e0004c 100644
--- a/org.eclipse.text/src/org/eclipse/jface/text/source/AnnotationModel.java
+++ b/org.eclipse.text/src/org/eclipse/jface/text/source/AnnotationModel.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2015 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -11,18 +11,19 @@
* Contributors:
* IBM Corporation - initial API and implementation
* Anton Leherbauer <anton.leherbauer@windriver.com> - [implementation] AnnotationModel.fModificationStamp leaks annotations - http://bugs.eclipse.org/345715
+ * Sebastian Zarnekow - [bug 401391] ConcurrentModificationException in AnnotationModel.getAnnotationIterator
*******************************************************************************/
package org.eclipse.jface.text.source;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.NoSuchElementException;
+import java.util.concurrent.ConcurrentHashMap;
import org.eclipse.core.runtime.Assert;
@@ -281,7 +282,7 @@ public class AnnotationModel implements IAnnotationModel, IAnnotationModelExtens
* The model's attachment.
* @since 3.0
*/
- private Map<Object, IAnnotationModel> fAttachments= new HashMap<>();
+ private Map<Object, IAnnotationModel> fAttachments= new ConcurrentHashMap<>();
/**
* The annotation model listener on attached sub-models.
* @since 3.0

Back to the top