Fix: ConcurrentModificationException when working with multiple xtext editors that reference each other

Fix: MemoryLeak in Linker (clearReferences was not called after reparse)

3 files changed, 35 insertions, 34 deletions

diff --git a/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocument.java b/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocument.java
index 7daad1f..59b3117 100644
--- a/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocument.java
+++ b/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocument.java
@@ -33,8 +33,8 @@ import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.emf.common.util.EList;
 import org.eclipse.emf.common.util.URI;
 import org.eclipse.emf.common.util.WrappedException;
+import org.eclipse.emf.ecore.EValidator;
 import org.eclipse.emf.ecore.resource.Resource;
-import org.eclipse.emf.ecore.util.Diagnostician;
 import org.eclipse.jdt.core.IJavaProject;
 import org.eclipse.jdt.core.JavaCore;
 import org.eclipse.jface.text.Document;
@@ -84,7 +84,7 @@ public class XtextDocument extends Document implements IXtextDocument {
 			}
 		}
 	}
-	
+
 	public boolean isReferenced(IResource anIResource) {
 		if (!(anIResource instanceof IFile))
 			return false;
@@ -96,7 +96,7 @@ public class XtextDocument extends Document implements IXtextDocument {
 				uriToRes.put(uri.lastSegment(), res);
 			}
 		}
-		
+
 		return uriToRes.containsKey(anIResource.getFullPath().lastSegment());
 	}
 
@@ -107,16 +107,16 @@ public class XtextDocument extends Document implements IXtextDocument {
 		return null;
 	}
 
-	private ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();
-	private Lock writeLock = rwLock.writeLock();
-	private Lock readLock = rwLock.readLock();
+	private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();
+	private final Lock writeLock = rwLock.writeLock();
+	private final Lock readLock = rwLock.readLock();
 
 	public <T> T readOnly(UnitOfWork<T> work) {
 		readLock.lock();
 		try {
 			updateContentBeforeRead();
 			T exec = work.exec(resource);
-			ensureThatStateIsNotReturned((Object) exec, work);
+			ensureThatStateIsNotReturned(exec, work);
 			return exec;
 		} catch (RuntimeException e) {
 			throw e;
@@ -131,7 +131,7 @@ public class XtextDocument extends Document implements IXtextDocument {
 		writeLock.lock();
 		try {
 			T exec = work.exec(resource);
-			ensureThatStateIsNotReturned((Object) exec, work);
+			ensureThatStateIsNotReturned(exec, work);
 			notifyModelListeners(resource);
 			// TODO track modifications and serialize back to the text buffer
 			return exec;
@@ -176,7 +176,7 @@ public class XtextDocument extends Document implements IXtextDocument {
 		}
 	}
 
-	private ListenerList xtextDocumentObservers = new ListenerList(ListenerList.IDENTITY);
+	private final ListenerList xtextDocumentObservers = new ListenerList(ListenerList.IDENTITY);
 
 	public void addXtextDocumentContentObserver(IXtextDocumentContentObserver observer) {
 		addDocumentListener(observer);
@@ -256,16 +256,16 @@ public class XtextDocument extends Document implements IXtextDocument {
 					readLock.lock();
 					writeLock.unlock();
 				}
-			} else
-				return null;
+			}
+			return null;
 		}
 
 	}
 
 	private static final Logger log = Logger.getLogger(XtextDocument.class);
-	private static final String MARKER_ID = Diagnostician.MARKER;
+	private static final String MARKER_ID = EValidator.MARKER;
 //	private static final String XTEXT_PARSEERROR_MARKER_TYPE = Activator.PLUGIN_ID + ".problemmarker";
-	private UpdateMarkerJob updateMarkerJob = new UpdateMarkerJob("updateMarkers");
+	private final UpdateMarkerJob updateMarkerJob = new UpdateMarkerJob("updateMarkers");
 
 	private void checkAndUpdateMarkers() {
 		updateMarkerJob.schedule();
@@ -276,9 +276,8 @@ public class XtextDocument extends Document implements IXtextDocument {
 		URI uri = resource.getURI();
 		if ((adapterType == IFile.class || adapterType == IResource.class) && uri.isPlatformResource()) {
 			return (T) ResourcesPlugin.getWorkspace().getRoot().getFile(new Path(uri.toPlatformString(true)));
-		} else {
-			return null;
 		}
+		return null;
 	}
 
 }
diff --git a/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocumentProvider.java b/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocumentProvider.java
index e03deb7..353f8f6 100644
--- a/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocumentProvider.java
+++ b/plugins/org.eclipse.xtext.ui.core/src/org/eclipse/xtext/ui/core/editor/model/XtextDocumentProvider.java
@@ -25,24 +25,24 @@ import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.emf.ecore.resource.Resource;
+import org.eclipse.emf.ecore.resource.ResourceSet;
 import org.eclipse.jface.text.IDocument;
 import org.eclipse.ui.IFileEditorInput;
 import org.eclipse.ui.editors.text.FileDocumentProvider;
-import org.eclipse.ui.texteditor.IDocumentProvider;
 import org.eclipse.xtext.resource.XtextResource;
 
 /**
  * @author Peter Friese - Initial contribution and API
  * @author Sven Efftinge
  */
-public class XtextDocumentProvider extends FileDocumentProvider implements IDocumentProvider {
+public class XtextDocumentProvider extends FileDocumentProvider {
 
 	private static final Logger log = Logger.getLogger(XtextDocumentProvider.class);
-	
+
 	/**
 	 * @author Sven Efftinge - Initial contribution and API
 	 *
-	 * updates referenced EMF Resources on IResourceChangeEvent 
+	 * updates referenced EMF Resources on IResourceChangeEvent
 	 */
 	private final class ReferencedResourcesUpdater implements IResourceChangeListener {
 		private final XtextDocument document;
@@ -50,32 +50,33 @@ public class XtextDocumentProvider extends FileDocumentProvider implements IDocu
 		private ReferencedResourcesUpdater(XtextDocument document) {
 			this.document = document;
 		}
-		
+
 		public void resourceChanged(final IResourceChangeEvent event) {
-			
+
 			final ResourceDeltaVisitor visitor = new ResourceDeltaVisitor(document);
 			try {
 				event.getDelta().accept(visitor);
-			} catch (CoreException e1) {
-				log.error(e1.getMessage(), e1);
+			} catch (CoreException e) {
+				log.error(e.getMessage(), e);
 			}
 			if (!visitor.deltas.isEmpty()) {
 			new Job("updating resourceset"){
 				@Override
 				protected IStatus run(IProgressMonitor monitor) {
 					document.modify(new UnitOfWork<Object>() {
-						
+
 						public Object exec(XtextResource arg) throws Exception {
 							for (IResourceDelta delta : visitor.deltas) {
 								IResource res = delta.getResource();
 								String string = res.getFullPath().lastSegment();
-								for (final Resource emfResource : arg.getResourceSet().getResources()) {
+								ResourceSet set = arg.getResourceSet();
+								for(int i = 0; i < set.getResources().size(); ) {
+									final Resource emfResource = set.getResources().get(i);
 									if (emfResource.getURI().lastSegment().equals(string)) {
 										switch (delta.getKind()) {
 										case IResourceDelta.REMOVED:
 											// UNLOAD
 											document.modify(new UnitOfWork<Object>() {
-												
 												public Object exec(XtextResource arg) throws Exception {
 													emfResource.unload();
 													return null;
@@ -98,6 +99,8 @@ public class XtextDocumentProvider extends FileDocumentProvider implements IDocu
 											break;
 										}
 									}
+									if (set.getResources().get(i) == emfResource)
+										i++;
 								}
 							}
 							arg.reparse(document.get());
@@ -107,7 +110,6 @@ public class XtextDocumentProvider extends FileDocumentProvider implements IDocu
 					return Status.OK_STATUS;
 				}}.schedule();
 			}
-
 		}
 	}
 
@@ -121,15 +123,15 @@ public class XtextDocumentProvider extends FileDocumentProvider implements IDocu
 		private ResourceDeltaVisitor(XtextDocument document) {
 			this.document = document;
 		}
-		
+
 		public final List<IResourceDelta> deltas = new ArrayList<IResourceDelta>();
 
 		public boolean visit(IResourceDelta delta) throws CoreException {
 			IResource res = delta.getResource();
 			int kind = delta.getKind();
 			int flags = delta.getFlags();
-			if ((kind == IResourceDelta.REMOVED ||  
-					(kind == IResourceDelta.CHANGED && ((IResourceDelta.CONTENT & flags) != 0))) 
+			if ((kind == IResourceDelta.REMOVED ||
+					(kind == IResourceDelta.CHANGED && ((IResourceDelta.CONTENT & flags) != 0)))
 					&& document.isReferenced(res)) {
 				deltas.add(delta);
 			}
diff --git a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/parsetree/NodeModelTest.java b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/parsetree/NodeModelTest.java
index 4781e07..a080141 100644
--- a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/parsetree/NodeModelTest.java
+++ b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/parsetree/NodeModelTest.java
@@ -37,7 +37,7 @@ public class NodeModelTest extends AbstractGeneratorTest {
 	public void testNavigabilityNode2Ast() throws Exception {
 		EObject object = getModel(MODEL);
 		EList<Adapter> adapters = object.eAdapters();
-		assert (adapters.size() == 1);
+		assertEquals(1 /* nodeAdapter */ + 1 /* clearer */, adapters.size());
 		NodeAdapter adapter = (NodeAdapter) adapters.get(0);
 		CompositeNode rootNode = adapter.getParserNode();
 		assertTrue(rootNode.eContainer() == null);
@@ -68,7 +68,7 @@ public class NodeModelTest extends AbstractGeneratorTest {
 
 	private void checkNavigabilityAst2Node(EObject object) {
 		EList<Adapter> adapters = object.eAdapters();
-		assertEquals(1, adapters.size());
+		assertEquals(1 /* nodeAdapter */ + 1 /* clearer */, adapters.size());
 		NodeAdapter adapter = (NodeAdapter) adapters.get(0);
 		AbstractNode parsetreeNode = adapter.getParserNode();
 		assertEquals(object, parsetreeNode.getElement());
@@ -78,7 +78,7 @@ public class NodeModelTest extends AbstractGeneratorTest {
 		EObject astElement = node.getElement();
 		if (astElement != null) {
 			EList<Adapter> adapters = astElement.eAdapters();
-			assertEquals(1, adapters.size());
+			assertEquals(1 /* nodeAdapter */ + 1 /* clearer */, adapters.size());
 			NodeAdapter adapter = (NodeAdapter) adapters.get(0);
 			assertEquals(node, adapter.getParserNode());
 		} else {
@@ -104,7 +104,7 @@ public class NodeModelTest extends AbstractGeneratorTest {
 			}
 		}
 	}
-	
+
 	public void testKeywordInAlternative() throws Exception {
 		with(SimpleExpressionsTestLanguageStandaloneSetup.class);
 		EObject object = getModel("d / e");