Fix deadlock in delta handling when multiple CVS operations are activeI20030923

6 files changed, 353 insertions, 122 deletions

diff --git a/bundles/org.eclipse.team.core/src/org/eclipse/team/core/subscribers/BackgroundEventHandler.java b/bundles/org.eclipse.team.core/src/org/eclipse/team/core/subscribers/BackgroundEventHandler.java
new file mode 100644
index 000000000..74d0f5958
--- /dev/null
+++ b/bundles/org.eclipse.team.core/src/org/eclipse/team/core/subscribers/BackgroundEventHandler.java
@@ -0,0 +1,230 @@
+/*******************************************************************************
+ * Copyright (c) 2000, 2003 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials 
+ * are made available under the terms of the Common Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/cpl-v10.html
+ * 
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.team.core.subscribers;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.jobs.IJobChangeEvent;
+import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.core.runtime.jobs.JobChangeAdapter;
+import org.eclipse.team.core.TeamException;
+import org.eclipse.team.internal.core.ExceptionCollector;
+import org.eclipse.team.internal.core.Policy;
+import org.eclipse.team.internal.core.TeamPlugin;
+
+/**
+ * Thsi class provides the infrastucture for processing events in the background 
+ */
+public abstract class BackgroundEventHandler {
+	
+	// Events that need to be processed
+	private List awaitingProcessing = new ArrayList();
+	
+	// The job that runs when events need to be processed
+	private Job eventHandlerJob;
+	
+	// Indicate if the event handler has benn shutdown
+	private boolean shutdown;
+
+	// manages exceptions
+	private ExceptionCollector errors;
+	
+	/**
+	 * Internal resource synchronization event. Can contain a result.
+	 */
+	public static class Event {
+		IResource resource;
+		int type;
+		int depth;
+		public Event(IResource resource, int type, int depth) {
+			this.resource = resource;
+			this.type = type;
+			this.depth = depth;
+		}
+		public int getDepth() {
+			return depth;
+		}
+		public IResource getResource() {
+			return resource;
+		}
+		public int getType() {
+			return type;
+		}
+	}
+	
+	
+	protected BackgroundEventHandler() {
+		errors =
+			new ExceptionCollector(
+				getErrorsTitle(),
+				TeamPlugin.ID,
+				IStatus.ERROR,
+				null /* don't log */
+		);
+	}
+
+	/**
+	 * Create the event handling job and schedule it
+	 */
+	protected void initializeEventHandlingJob() {
+		createEventHandlingJob();
+		schedule();
+	}
+	
+	/**
+	 * Handle the exception by recording it in the errors list.
+	 * @param e
+	 */
+	protected void handleException(TeamException e) {
+		errors.handleException(e);
+		
+	}
+	/**
+	 * Create the job used for processing the events in the queue. The job stops working when
+	 * the queue is empty.
+	 */
+	protected void createEventHandlingJob() {
+		eventHandlerJob = new Job(getName()) {	
+			public IStatus run(IProgressMonitor monitor) {
+				return processEvents(monitor);
+			}
+		};
+		eventHandlerJob.addJobChangeListener(new JobChangeAdapter() {
+			public void done(IJobChangeEvent event) {
+				jobDone();
+			}
+		});
+		eventHandlerJob.setPriority(Job.SHORT);
+	}
+
+	/**
+	 * Return the name of the handler, which is used as the job name.
+	 * @return the name of the handler
+	 */
+	public abstract String getName();
+	
+	/**
+	 * Return the text to be displayed as the title for any errors that occur.
+	 * @return
+	 */
+	public abstract String getErrorsTitle();
+	
+	/**
+	 * Process the event in the context of a background job.
+	 * 
+	 * @param event
+	 * @param monitor
+	 */
+	protected abstract void processEvent(Event event, IProgressMonitor monitor) throws TeamException;
+	
+	/**
+	 * Shutdown the event handler. Any events on the queue will be removed from the queue
+	 * and will not be processed.
+	 */
+	public void shutdown() {
+		shutdown = true;
+		eventHandlerJob.cancel();
+	}
+	
+	/**
+	 * Queue the event and start the job if it's not already doing work.
+	 */
+	protected synchronized void queueEvent(Event event) {
+		awaitingProcessing.add(event);
+		if (shutdown
+			|| eventHandlerJob == null
+			|| eventHandlerJob.getState() != Job.NONE)
+			return;
+		else {
+			schedule();
+		}
+	}
+	
+	/**
+	 * Get the next resource to be calculated.
+	 * @return Event to be processed
+	 */
+	private synchronized Event nextElement() {
+		if (shutdown || awaitingProcessing.isEmpty()) {
+			return null;
+		}
+		return (Event) awaitingProcessing.remove(0);
+	}
+	
+	/**
+	 * Process events from the events queue and dispatch results. 
+	 */
+	protected IStatus processEvents(IProgressMonitor monitor) {
+		Event event;
+		errors.clear();
+		try {
+			// It's hard to know how much work is going to happen
+			// since the queue can grow. Use the current queue size as a hint to
+			// an infinite progress monitor
+			monitor.beginTask(null, 100);
+			IProgressMonitor subMonitor = Policy.infiniteSubMonitorFor(monitor, 90);
+			subMonitor.beginTask(null, 1024);
+
+			while ((event = nextElement()) != null) {
+				// Cancellation is dangerous because this will leave the sync info in a bad state.
+				// Purposely not checking -				 	
+				try {
+					processEvent(event, subMonitor);
+				} catch (TeamException e) {
+					// handle exception but keep going
+					handleException(e);
+				}
+			}
+		} finally {
+			monitor.done();
+		}
+		return errors.getStatus();
+	}
+
+	/**
+	 * This method is invoked when the processing job completes. The
+	 * default behavior of the handler is to restart the job if the queue
+	 * is no longer empty and to clear the queue if the handler was shut down.
+	 *
+	 */
+	protected void jobDone() {
+		// Make sure an unhandled event didn't squeak in unless we are shutdown
+		if (shutdown == false && hasUnprocessedEvents()) {
+			schedule();
+		} else {
+			synchronized(this) {
+				awaitingProcessing.clear();
+			}
+		}
+	}
+	protected boolean hasUnprocessedEvents() {
+		return !awaitingProcessing.isEmpty();
+	}
+	
+	/**
+	 * Schedule the job or process the events now.
+	 */
+	protected void schedule() {
+		eventHandlerJob.schedule();
+	}
+
+	/**
+	 * @return Returns the eventHandlerJob.
+	 */
+	public Job getEventHandlerJob() {
+		return eventHandlerJob;
+	}
+
+}
diff --git a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/messages.properties b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/messages.properties
index 3dc92dcae..8572fb245 100644
--- a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/messages.properties
+++ b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/messages.properties
@@ -331,3 +331,5 @@ RemoteTagSynchronizer.0=Refreshing {0}
 ReentrantLock.9=An error occurred writting CVS synchronization information to disk. Some information may be lost.
 CRLFDetectInputStream.0=CVS file {0} either contains invalid line endings on the server (CR/LF instead of just LF) or is a binary file that is not marked as -kb.
 SynchronizerSyncInfoCache.0=Synchronization information could not be cached for {0}. The only negative effect of this may be decreased performance.
+DeferredResourceChangeHandler.0=Reconciling CVS state changes
+DeferredResourceChangeHandler.1=Errors occured handling ignore file (.cvsignore) changes. Some resources may not be decorated properly.
diff --git a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/EclipseSynchronizer.java b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/EclipseSynchronizer.java
index ff124e310..d13a2e72b 100644
--- a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/EclipseSynchronizer.java
+++ b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/EclipseSynchronizer.java
@@ -906,11 +906,6 @@ public class EclipseSynchronizer implements IFlushOperation {
 			allChanges.addAll(Arrays.asList(changedResources));
 			allChanges.addAll(Arrays.asList(changedFolders));
 			allChanges.addAll(dirtyParents);	
-			try {
-				allChanges.addAll(Arrays.asList(getResourcesAffectedByChangedIgnoreFiles(threadInfo.getChangedIgnoreFiles())));
-			} catch (CVSException e) {
-				errors.add(e.getStatus());
-			}
 			IResource[] resources = (IResource[]) allChanges.toArray(
 				new IResource[allChanges.size()]);
 			broadcastResourceStateChanges(resources);
@@ -1466,77 +1461,16 @@ public class EclipseSynchronizer implements IFlushOperation {
 	}
 
 	/**
-	 * Return true if the given resource is contained by the scheduling rule
-	 * that is being used by the current thread.
+	 * Return whether the resource is within the scope of a currently active
+	 * CVS operation.
 	 * @param resource
 	 * @return
 	 */
-	public boolean isWithinScopeOfActiveOperation(IResource resource) {
-		return resourceLock.isWithinActiveThread(resource);
-	}
-
-	/**
-	 * Record the changed ignore file so it can be handled at the
-	 * end of the operation associated with the current thread.
-	 * @param resource
-	 */
-	public void handleIgnoreFileChange(IResource resource) {
-		Assert.isTrue(resource.getType() == IResource.FILE);
-		resourceLock.recordIgnoreFileChange((IFile)resource);
-	}
-	
-	private IResource[] getResourcesAffectedByChangedIgnoreFiles(IFile[] files) throws CVSException {
-		try {
-			Set changedPeers = new HashSet();
-			for (int i = 0; i < files.length; i++) {
-				IContainer parent = files[i].getParent();
-				if (parent.exists()) {
-					// Include the parent
-					changedPeers.add(parent);
-					// Include all siblings
-					changedPeers.addAll(Arrays.asList(parent.members(false)));
-				}
-			}
-			return (IResource[]) changedPeers.toArray(new IResource[changedPeers.size()]);
-		} catch (CoreException e) {
-			throw CVSException.wrapException(e);
-		}
+	public boolean isWithinActiveOperationScope(IResource resource) {
+		// TODO Auto-generated method stub
+		return false;
 	}
 	
-	/**
-	 * Handle a change event on the given resource. If the resource has been changed
-	 * as part of a currently running CVS operation, then add the resource change to
-	 * the list of changed resources for the operation (if necessary) and return <code>true</code>.
-	 * Otherwise, return <code>false</code> (indicating that the client should handle the change).
-	 * @param resource
-	 * @return
-	 */
-	public boolean handleResourceChanged(IResource resource) {
-		try {
-			beginOperation();
-			if(isWithinScopeOfActiveOperation(resource)) {
-				// This resource is modified by an active CVS operation
-				// (i.e. this is an intermitant delta)
-				// Notification is not required (or possible) in this case
-				// as the operation will peform the necessary notifications
-				// when it completes
-				if (isIgnoreFile(resource)) {
-					// Add the ignore file to the current operation 
-					// so it is handled when the operation completes
-					handleIgnoreFileChange(resource);
-				}
-				// Indicate that we've handled the resource
-				return true;
-			}
-			// Indicate that the client shoudl handle the change
-			return false;
-		} finally {
-			endOperation();
-		}
-	}
+
 	
-	private boolean isIgnoreFile(IResource resource) {
-		return resource.getType() == IResource.FILE &&
-			resource.getName().equals(SyncFileWriter.IGNORE_FILE);
-	}
 }
diff --git a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/syncinfo/DeferredResourceChangeHandler.java b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/syncinfo/DeferredResourceChangeHandler.java
new file mode 100644
index 000000000..a57bdad8b
--- /dev/null
+++ b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/syncinfo/DeferredResourceChangeHandler.java
@@ -0,0 +1,82 @@
+/*******************************************************************************
+ * Copyright (c) 2000, 2003 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials 
+ * are made available under the terms of the Common Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/cpl-v10.html
+ * 
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.team.internal.ccvs.core.syncinfo;
+
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
+import org.eclipse.core.resources.IContainer;
+import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.team.core.TeamException;
+import org.eclipse.team.core.subscribers.BackgroundEventHandler;
+import org.eclipse.team.internal.ccvs.core.resources.EclipseSynchronizer;
+import org.eclipse.team.internal.ccvs.core.Policy;
+
+/**
+ * This class handles resources changes that are reported in deltas
+ * in a deferred manner (i.e. in a background job)
+ */
+public class DeferredResourceChangeHandler extends BackgroundEventHandler {
+
+	private static final int IGNORE_FILE_CHANGED = 1;
+	
+	private Set changedIgnoreFiles = new HashSet();
+
+	private int NOTIFICATION_BATCHING_NUMBER = 10;
+
+	/* (non-Javadoc)
+	 * @see org.eclipse.team.core.subscribers.BackgroundEventHandler#getName()
+	 */
+	public String getName() {
+		return Policy.bind("DeferredResourceChangeHandler.0"); //$NON-NLS-1$
+	}
+
+	/* (non-Javadoc)
+	 * @see org.eclipse.team.core.subscribers.BackgroundEventHandler#getErrorsTitle()
+	 */
+	public String getErrorsTitle() {
+		return Policy.bind("DeferredResourceChangeHandler.1"); //$NON-NLS-1$
+	}
+
+	/* (non-Javadoc)
+	 * @see org.eclipse.team.core.subscribers.BackgroundEventHandler#processEvent(org.eclipse.team.core.subscribers.BackgroundEventHandler.Event, org.eclipse.core.runtime.IProgressMonitor)
+	 */
+	protected void processEvent(Event event, IProgressMonitor monitor) throws TeamException {
+		int type = event.getType();
+		switch (type) {
+			case IGNORE_FILE_CHANGED :
+				changedIgnoreFiles.add(event.getResource());
+		}
+		
+		if (!hasUnprocessedEvents()
+			|| changedIgnoreFiles.size() > NOTIFICATION_BATCHING_NUMBER) {
+			EclipseSynchronizer.getInstance().syncFilesChanged(getParents(changedIgnoreFiles));
+			changedIgnoreFiles.clear();
+		}
+	}
+	
+	private IContainer[] getParents(Set files) {
+		Set parents = new HashSet();
+		for (Iterator iter = files.iterator(); iter.hasNext();) {
+			IFile file = (IFile) iter.next();
+			parents.add(file.getParent());
+		}
+		return (IContainer[]) parents.toArray(new IContainer[parents.size()]);
+	}
+
+	public void ignoreFileChanged(IFile file) {
+		queueEvent(new Event(file, IGNORE_FILE_CHANGED, IResource.DEPTH_ZERO));
+	}
+	
+}
diff --git a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/syncinfo/ReentrantLock.java b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/syncinfo/ReentrantLock.java
index f03a3f3c2..447aa6f45 100644
--- a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/syncinfo/ReentrantLock.java
+++ b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/syncinfo/ReentrantLock.java
@@ -61,7 +61,6 @@ public class ReentrantLock {
 	public class ThreadInfo {
 		private Set changedResources = new HashSet();
 		private Set changedFolders = new HashSet();
-		private Set changedIgnoreFiles = new HashSet();
 		private IFlushOperation operation;
 		private List rules = new ArrayList();
 		public ThreadInfo(IFlushOperation operation) {
@@ -130,11 +129,8 @@ public class ReentrantLock {
 		public void addChangedFolder(IContainer container) {
 			changedFolders.add(container);
 		}
-		public void addChangedIgnoreFile(IFile resource) {
-			changedIgnoreFiles.add(resource);
-		}
 		public boolean isEmpty() {
-			return changedFolders.isEmpty() && changedResources.isEmpty() && changedIgnoreFiles.isEmpty();
+			return changedFolders.isEmpty() && changedResources.isEmpty();
 		}
 		public IResource[] getChangedResources() {
 			return (IResource[]) changedResources.toArray(new IResource[changedResources.size()]);
@@ -142,9 +138,6 @@ public class ReentrantLock {
 		public IContainer[] getChangedFolders() {
 			return (IContainer[]) changedFolders.toArray(new IContainer[changedFolders.size()]);
 		}
-		public IFile[] getChangedIgnoreFiles() {
-			return (IFile[]) changedIgnoreFiles.toArray(new IFile[changedIgnoreFiles.size()]);
-		}
 		public void flush(IProgressMonitor monitor) throws CVSException {
 			try {
 				operation.flush(this, monitor);
@@ -205,22 +198,26 @@ public class ReentrantLock {
 	}
 	
 	private ThreadInfo getThreadInfo(IResource resource) {
-		for (Iterator iter = infos.values().iterator(); iter.hasNext();) {
-			ThreadInfo info = (ThreadInfo) iter.next();
-			if (info.ruleContains(resource)) {
-				return info;
+		synchronized (infos) {
+			for (Iterator iter = infos.values().iterator(); iter.hasNext();) {
+				ThreadInfo info = (ThreadInfo) iter.next();
+				if (info.ruleContains(resource)) {
+					return info;
+				}
 			}
+			return null;
 		}
-		return null;
 	}
 	
 	public synchronized void acquire(IResource resource, IFlushOperation operation) {
 		ThreadInfo info = getThreadInfo();
-		if (info == null) {
-			info = new ThreadInfo(operation);
-			Thread thisThread = Thread.currentThread();
-			infos.put(thisThread, info);
-			if(DEBUG) System.out.println("[" + thisThread.getName() + "] acquired CVS lock on " + resource.getFullPath()); //$NON-NLS-1$ //$NON-NLS-2$
+		synchronized (infos) {
+			if (info == null) {
+				info = new ThreadInfo(operation);
+				Thread thisThread = Thread.currentThread();
+				infos.put(thisThread, info);
+				if(DEBUG) System.out.println("[" + thisThread.getName() + "] acquired CVS lock on " + resource.getFullPath()); //$NON-NLS-1$ //$NON-NLS-2$
+			}
 		}
 		info.pushRule(resource);
 	}
@@ -236,10 +233,12 @@ public class ReentrantLock {
 		Assert.isNotNull(info, "Unmatched acquire/release."); //$NON-NLS-1$
 		Assert.isTrue(info.isNested(), "Unmatched acquire/release."); //$NON-NLS-1$
 		info.popRule(resource, monitor);
-		if (!info.isNested()) {
-			Thread thisThread = Thread.currentThread();
-			if(DEBUG) System.out.println("[" + thisThread.getName() + "] released CVS lock"); //$NON-NLS-1$ //$NON-NLS-2$
-			infos.remove(thisThread);
+		synchronized (infos) {
+			if (!info.isNested()) {
+				Thread thisThread = Thread.currentThread();
+				if(DEBUG) System.out.println("[" + thisThread.getName() + "] released CVS lock"); //$NON-NLS-1$ //$NON-NLS-2$
+				infos.remove(thisThread);
+			}
 		}
 	}
 
@@ -264,23 +263,9 @@ public class ReentrantLock {
 		info.flush(monitor);
 	}
 
-	/**
-	 * Return <code>true</code> if the current thread is part of a CVS operation
-	 * and the given resource is contained the scheduling rule held by that operation.
-	 * @param resource
-	 * @return
-	 */
-	public synchronized boolean isWithinActiveThread(IResource resource) {
-		return getThreadInfo(resource) != null;
-	}
-
-	/**
-	 * Record the ignore file change as part of the current operation.
-	 * @param resource
-	 */
-	public synchronized void recordIgnoreFileChange(IFile resource) {
-		ThreadInfo info = getThreadInfo(resource);
-		Assert.isNotNull(info);
-		info.addChangedIgnoreFile(resource);
+	public boolean isWithinActiveOperationScope(IFile resource) {
+		synchronized (infos) {
+			return getThreadInfo(resource) != null;
+		}
 	}
 }
diff --git a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/util/SyncFileChangeListener.java b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/util/SyncFileChangeListener.java
index 3ccc07836..e5405078a 100644
--- a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/util/SyncFileChangeListener.java
+++ b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/util/SyncFileChangeListener.java
@@ -28,6 +28,7 @@ import org.eclipse.team.internal.ccvs.core.ICVSFile;
 import org.eclipse.team.internal.ccvs.core.Policy;
 import org.eclipse.team.internal.ccvs.core.resources.CVSWorkspaceRoot;
 import org.eclipse.team.internal.ccvs.core.resources.EclipseSynchronizer;
+import org.eclipse.team.internal.ccvs.core.syncinfo.DeferredResourceChangeHandler;
 
 /*
  * Listens to CVS meta-file changes and notifies the EclipseSynchronizer of changes made to sync files 
@@ -61,7 +62,9 @@ public class SyncFileChangeListener implements IResourceChangeListener {
 																	IResourceDelta.REPLACED |
 																	IResourceDelta.TYPE;
 	
-	protected boolean isProjectOpening = false;								
+	protected boolean isProjectOpening = false;
+	
+	protected DeferredResourceChangeHandler deferredHandler = new DeferredResourceChangeHandler();
 	
 	/*
 	 * When a resource changes this method will detect if the changed resources is a meta file that has changed 
@@ -111,20 +114,15 @@ public class SyncFileChangeListener implements IResourceChangeListener {
 					} else {
 						// Inform the synchronizer about folder creations
 						if(isProjectOpening()) return true;
-						// TODO: Can't do this in a POST_CHANGE
-//						if (kind == IResourceDelta.ADDED) {
-//							try {
-//								EclipseSynchronizer.getInstance().created(resource);
-//							} catch (CVSException e) {
-//								throw new CoreException(e.getStatus());
-//							}
-//						}
 					}
 					
-					if (EclipseSynchronizer.getInstance().handleResourceChanged(resource)) {
+					if (EclipseSynchronizer.getInstance().isWithinActiveOperationScope(resource)) {
 						// The resource change will be handled by the EclipseSynchronizer
 						// Still visit the children of non-team-private members so that 
 						// ignore file changes will be past on to the EclipseSynchronizer
+						if (isIgnoreFile(resource)) {
+							deferredHandler.ignoreFileChanged((IFile)resource);
+						}
 						return (!resource.isTeamPrivateMember());
 					} if(isMetaFile(resource)) {
 						toBeNotified = handleChangedMetaFile(resource, kind);