Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJean Michel-Lemieux2003-08-19 10:11:41 -0400
committerJean Michel-Lemieux2003-08-19 10:11:41 -0400
commit5ab9f57711a0c566b20958cc2483fab12dc295ed (patch)
tree83e5657ac895a374473065089d39b574388dded3
parentcbe98b8b3c6e3948a253d5e0dbd2cfdb4c3d96a4 (diff)
downloadeclipse.platform.team-5ab9f57711a0c566b20958cc2483fab12dc295ed.tar.gz
eclipse.platform.team-5ab9f57711a0c566b20958cc2483fab12dc295ed.tar.xz
eclipse.platform.team-5ab9f57711a0c566b20958cc2483fab12dc295ed.zip
Added deterministic locking to CVS sync info cache using the new job API.
-rw-r--r--bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/EclipseSynchronizer.java83
-rw-r--r--bundles/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/resources/SynchronizerSyncInfoCache.java14
-rw-r--r--bundles/org.eclipse.team.cvs.ui/src/org/eclipse/team/internal/ccvs/ui/CVSLightweightDecorator.java6
-rw-r--r--tests/org.eclipse.team.tests.cvs.core/src/org/eclipse/team/tests/ccvs/core/cvsresources/BatchedTestSetup.java4
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 {

Back to the top