diff options
4 files changed, 39 insertions, 68 deletions
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 819a6aa60..0a1d7bc88 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 @@ -27,6 +27,8 @@ import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; import org.eclipse.core.resources.IResourceStatus; import org.eclipse.core.resources.IResourceVisitor; +import org.eclipse.core.resources.IWorkspaceRoot; +import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; @@ -55,6 +57,12 @@ import org.eclipse.team.internal.ccvs.core.util.SyncFileWriter; * A synchronizer is responsible for managing synchronization information for local * CVS resources. * + * This class is thread safe but only allows one thread to modify the cache at a time. It + * doesn't support fine grain locking on a resource basis. Lock ordering between the workspace + * lock and the synchronizer lock is guaranteed to be deterministic. That is, the workspace + * lock is *always* acquired before the synchronizer lock. This protects against possible + * deadlock cases where the synchronizer lock is acquired before a workspace lock. + * * Special processing has been added for linked folders and their childen so * that their CVS meta files are never read or written. * @@ -123,7 +131,7 @@ public class EclipseSynchronizer { Policy.bind("EclipseSynchronizer.ErrorSettingFolderSync", folder.getFullPath().toString())); //$NON-NLS-1$ } try { - beginOperation(null); + beginOperation(folder, null); // get the old info FolderSyncInfo oldInfo = getFolderSync(folder); // set folder sync and notify @@ -148,7 +156,7 @@ public class EclipseSynchronizer { public FolderSyncInfo getFolderSync(IContainer folder) throws CVSException { if (folder.getType() == IResource.ROOT || !isValid(folder)) return null; try { - beginOperation(null); + beginOperation(folder, null); cacheFolderSync(folder); return getSyncInfoCacheFor(folder).getCachedFolderSync(folder); } finally { @@ -166,7 +174,7 @@ public class EclipseSynchronizer { public void deleteFolderSync(IContainer folder) throws CVSException { if (folder.getType() == IResource.ROOT || !isValid(folder)) return; try { - beginOperation(null); + beginOperation(folder, null); // iterate over all children with sync info and prepare notifications // this is done first since deleting the folder sync may remove a phantom cacheResourceSyncForChildren(folder); @@ -203,7 +211,7 @@ public class EclipseSynchronizer { Policy.bind("EclipseSynchronizer.ErrorSettingResourceSync", resource.getFullPath().toString())); //$NON-NLS-1$ } try { - beginOperation(null); + beginOperation(resource, null); // cache resource sync for siblings, set for self, then notify cacheResourceSyncForChildren(parent); setCachedResourceSync(resource, info); @@ -237,7 +245,7 @@ public class EclipseSynchronizer { IContainer parent = resource.getParent(); if (parent == null || parent.getType() == IResource.ROOT || !isValid(parent)) return null; try { - beginOperation(null); + beginOperation(resource, null); // cache resource sync for siblings, then return for self try { cacheResourceSyncForChildren(parent); @@ -272,7 +280,7 @@ public class EclipseSynchronizer { Policy.bind("EclipseSynchronizer.ErrorSettingResourceSync", resource.getFullPath().toString())); //$NON-NLS-1$ } try { - beginOperation(null); + beginOperation(resource, null); // cache resource sync for siblings, set for self, then notify cacheResourceSyncForChildren(parent); setCachedSyncBytes(resource, syncBytes); @@ -292,7 +300,7 @@ public class EclipseSynchronizer { IContainer parent = resource.getParent(); if (parent == null || parent.getType() == IResource.ROOT || !isValid(parent)) return; try { - beginOperation(null); + beginOperation(resource, null); // cache resource sync for siblings, delete for self, then notify cacheResourceSyncForChildren(parent); if (getCachedSyncBytes(resource) != null) { // avoid redundant notifications @@ -327,7 +335,7 @@ public class EclipseSynchronizer { return false; } try { - beginOperation(null); + beginOperation(resource, null); FileNameMatcher matcher = cacheFolderIgnores(resource.getParent()); return matcher.match(resource.getName()); } finally { @@ -347,7 +355,7 @@ public class EclipseSynchronizer { Policy.bind("EclipseSynchronizer.ErrorSettingIgnorePattern", folder.getFullPath().toString())); //$NON-NLS-1$ } try { - beginOperation(null); + beginOperation(folder, null); String[] ignores = SyncFileWriter.readCVSIgnoreEntries(folder); if (ignores != null) { // verify that the pattern has not already been added @@ -383,7 +391,7 @@ public class EclipseSynchronizer { public IResource[] members(IContainer folder) throws CVSException { if (! isValid(folder)) return new IResource[0]; try { - beginOperation(null); + beginOperation(folder, null); if (folder.getType() != IResource.ROOT) { // ensure that the sync info is cached so any required phantoms are created cacheResourceSyncForChildren(folder); @@ -401,7 +409,9 @@ public class EclipseSynchronizer { * * @param monitor the progress monitor, may be null */ - public void beginOperation(IProgressMonitor monitor) throws CVSException { + public void beginOperation(IResource resource, IProgressMonitor monitor) throws CVSException { + // ensure locks are acquired in the same order: workspace then cvs + Platform.getJobManager().beginRule(resource); lock.acquire(); if (lock.getDepth() == 1) { @@ -421,7 +431,7 @@ public class EclipseSynchronizer { * @exception CVSException with a status with code <code>COMMITTING_SYNC_INFO_FAILED</code> * if all the CVS sync information could not be written to disk. */ - public void endOperation(IProgressMonitor monitor) throws CVSException { + public void endOperation(IProgressMonitor monitor) throws CVSException { try { IStatus status = SyncInfoCache.STATUS_OK; if (lock.getDepth() == 1) { @@ -431,7 +441,9 @@ public class EclipseSynchronizer { throw new CVSException(status); } } finally { - lock.release(); + // ensure locks are released in the same order: cvs then workspace + lock.release(); + Platform.getJobManager().endRule(); } } @@ -459,7 +471,7 @@ public class EclipseSynchronizer { monitor = Policy.monitorFor(monitor); monitor.beginTask(null, 10); try { - beginOperation(Policy.subMonitorFor(monitor, 1)); + beginOperation(root, Policy.subMonitorFor(monitor, 1)); IStatus status = commitCache(Policy.subMonitorFor(monitor, 7)); @@ -528,7 +540,7 @@ public class EclipseSynchronizer { public void prepareForDeletion(IResource resource) throws CVSException { if (!resource.exists()) return; try { - beginOperation(null); + beginOperation(resource, null); // Flush the dirty info for the resource and it's ancestors. // Although we could be smarter, we need to do this because the // deletion may fail. @@ -573,7 +585,7 @@ public class EclipseSynchronizer { protected void handleDeleted(IResource resource) throws CVSException { if (resource.exists()) return; try { - beginOperation(null); + beginOperation(resource, null); adjustDirtyStateRecursively(resource, RECOMPUTE_INDICATOR); } finally { endOperation(null); @@ -635,7 +647,7 @@ public class EclipseSynchronizer { private void created(IFolder folder) throws CVSException { try { // set the dirty count using what was cached in the phantom it - beginOperation(null); + beginOperation(folder, null); FolderSyncInfo folderInfo = synchronizerCache.getCachedFolderSync(folder); byte[] syncBytes = synchronizerCache.getCachedSyncBytes(folder); if (folderInfo != null && syncBytes != null) { @@ -687,7 +699,7 @@ public class EclipseSynchronizer { private void created(IFile file) throws CVSException { try { // set the dirty count using what was cached in the phantom it - beginOperation(null); + beginOperation(file, null); byte[] syncBytes = synchronizerCache.getCachedSyncBytes(file); if (syncBytes == null) return; byte[] newBytes = getSyncBytes(file); @@ -802,10 +814,6 @@ public class EclipseSynchronizer { if (changedFolders.isEmpty() && changedResources.isEmpty()) { return SyncInfoCache.STATUS_OK; } - if (!isWorkspaceModifiable()) { - // if the workspace is closed for modification, we'll wait until the next commit - return SyncInfoCache.STATUS_OK; - } List errors = new ArrayList(); try { /*** prepare operation ***/ @@ -1245,7 +1253,7 @@ public class EclipseSynchronizer { for (int i = 0; i < folders.length; i++) { IContainer parent = folders[i]; try { - beginOperation(null); + beginOperation(parent, null); cacheResourceSyncForChildren(parent); cacheFolderSync(parent); cacheFolderIgnores(parent); @@ -1296,7 +1304,8 @@ public class EclipseSynchronizer { monitor = Policy.monitorFor(monitor); monitor.beginTask(null, 100); try { - beginOperation(Policy.subMonitorFor(monitor, 5)); + IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot(); + beginOperation(root, Policy.subMonitorFor(monitor, 5)); job.run(Policy.subMonitorFor(monitor, 60)); } finally { endOperation(Policy.subMonitorFor(monitor, 35)); @@ -1317,7 +1326,7 @@ public class EclipseSynchronizer { private void adjustDirtyStateRecursively(IResource resource, String indicator) throws CVSException { if (resource.getType() == IResource.ROOT) return; try { - beginOperation(null); + beginOperation(resource, null); if (indicator == getDirtyIndicator(resource)) { return; @@ -1348,7 +1357,7 @@ public class EclipseSynchronizer { protected String getDirtyIndicator(IResource resource) throws CVSException { try { - beginOperation(null); + beginOperation(resource, null); return getSyncInfoCacheFor(resource).getDirtyIndicator(resource); } finally { endOperation(null); @@ -1362,7 +1371,7 @@ public class EclipseSynchronizer { */ protected void setDirtyIndicator(IResource resource, boolean modified) throws CVSException { try { - beginOperation(null); + beginOperation(resource, null); String indicator = modified ? IS_DIRTY_INDICATOR : NOT_DIRTY_INDICATOR; // set the dirty indicator and adjust the parent accordingly adjustDirtyStateRecursively(resource, indicator); @@ -1450,24 +1459,4 @@ public class EclipseSynchronizer { return ICVSFile.UNKNOWN; } } - - /** - * If this method return false, the caller should not perform any workspace modification - * operations. The danger of performing such an operation is deadlock. - * - * @return boolean - */ - protected boolean isWorkspaceModifiable() { - return true; //!lock.isReadOnly(); - } - - /** - * Register the given thread as a thread that should be resitricted to having read-only access. - * If a thread is not registered, it is expected that they obtain the workspace lock before - * accessing any CVS sync information. - * @param thread - */ - public void addReadOnlyThread(Thread thread) { - //lock.addReadOnlyThread(thread); - } } diff --git a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/SynchronizerSyncInfoCache.java b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/SynchronizerSyncInfoCache.java index e9ae37b49..42640195f 100644 --- a/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/SynchronizerSyncInfoCache.java +++ b/bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/SynchronizerSyncInfoCache.java @@ -27,7 +27,6 @@ import org.eclipse.core.runtime.CoreException; import org.eclipse.team.internal.ccvs.core.CVSException; import org.eclipse.team.internal.ccvs.core.ICVSFolder; import org.eclipse.team.internal.ccvs.core.ICVSResource; -import org.eclipse.team.internal.ccvs.core.Policy; import org.eclipse.team.internal.ccvs.core.syncinfo.FolderSyncInfo; import org.eclipse.team.internal.ccvs.core.syncinfo.ResourceSyncInfo; import org.eclipse.team.internal.ccvs.core.util.SyncFileWriter; @@ -143,11 +142,9 @@ import org.eclipse.team.internal.ccvs.core.util.Util; try { if (info == null) { if (container.exists() || container.isPhantom()) { - ensureWorkspaceModifiable(container); getWorkspaceSynchronizer().flushSyncInfo(FOLDER_SYNC_KEY, container, IResource.DEPTH_ZERO); } } else { - ensureWorkspaceModifiable(container); getWorkspaceSynchronizer().setSyncInfo(FOLDER_SYNC_KEY, container, info.getBytes()); } } catch (CoreException e) { @@ -181,7 +178,6 @@ import org.eclipse.team.internal.ccvs.core.util.Util; // We do this to avoid causing a resource delta when the sync info is // initially loaded (i.e. the synchronizer has it and so does the Entries file if (oldBytes == null || !Util.equals(syncBytes, oldBytes)) { - ensureWorkspaceModifiable(resource); getWorkspaceSynchronizer().setSyncInfo(RESOURCE_SYNC_KEY, resource, syncBytes); } } @@ -275,16 +271,6 @@ import org.eclipse.team.internal.ccvs.core.util.Util; } /** - * - */ - protected void ensureWorkspaceModifiable(IResource resource) throws CVSException { - if (!EclipseSynchronizer.getInstance().isWorkspaceModifiable()) { - throw new CVSException(Policy.bind("EclipseSynchronizer.workspaceClosedForResource", resource.getFullPath().toString())); //$NON-NLS-1$ - } - - } - - /** * @param root * @param deep */ diff --git a/bundles/org.eclipse.team.cvs.ui/src/org/eclipse/team/internal/ccvs/ui/CVSLightweightDecorator.java b/bundles/org.eclipse.team.cvs.ui/src/org/eclipse/team/internal/ccvs/ui/CVSLightweightDecorator.java index e43dc7f0e..777db2ae4 100644 --- a/bundles/org.eclipse.team.cvs.ui/src/org/eclipse/team/internal/ccvs/ui/CVSLightweightDecorator.java +++ b/bundles/org.eclipse.team.cvs.ui/src/org/eclipse/team/internal/ccvs/ui/CVSLightweightDecorator.java @@ -44,7 +44,6 @@ import org.eclipse.team.internal.ccvs.core.ICVSResource; import org.eclipse.team.internal.ccvs.core.IResourceStateChangeListener; import org.eclipse.team.internal.ccvs.core.client.Command.KSubstOption; 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.FolderSyncInfo; import org.eclipse.team.internal.ccvs.core.syncinfo.ResourceSyncInfo; import org.eclipse.team.internal.core.ExceptionCollector; @@ -157,11 +156,6 @@ public class CVSLightweightDecorator */ public void decorate(Object element, IDecoration decoration) { - // Make sure that the decorator thread only has read access to the CVS sync info. - // This will register the thread on each decoration but it's the only way we - // know of to ensure the proper thread is registered. - EclipseSynchronizer.getInstance().addReadOnlyThread(Thread.currentThread()); - IResource resource = getResource(element); if (resource == null || resource.getType() == IResource.ROOT) return; diff --git a/tests/org.eclipse.team.tests.cvs.core/src/org/eclipse/team/tests/ccvs/core/cvsresources/BatchedTestSetup.java b/tests/org.eclipse.team.tests.cvs.core/src/org/eclipse/team/tests/ccvs/core/cvsresources/BatchedTestSetup.java index c833c942a..1cf00240b 100644 --- a/tests/org.eclipse.team.tests.cvs.core/src/org/eclipse/team/tests/ccvs/core/cvsresources/BatchedTestSetup.java +++ b/tests/org.eclipse.team.tests.cvs.core/src/org/eclipse/team/tests/ccvs/core/cvsresources/BatchedTestSetup.java @@ -13,6 +13,8 @@ package org.eclipse.team.tests.ccvs.core.cvsresources; import junit.extensions.TestSetup; import junit.framework.Test; + +import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.team.internal.ccvs.core.CVSException; import org.eclipse.team.internal.ccvs.core.resources.EclipseSynchronizer; @@ -22,7 +24,7 @@ public class BatchedTestSetup extends TestSetup { } public void setUp() throws CVSException { - EclipseSynchronizer.getInstance().beginOperation(null); + EclipseSynchronizer.getInstance().beginOperation(ResourcesPlugin.getWorkspace().getRoot(), null); } public void tearDown() throws CVSException { |