diff options
author | Eike Stepper | 2013-05-28 18:18:05 +0000 |
---|---|---|
committer | Eike Stepper | 2013-05-28 18:18:05 +0000 |
commit | 1c67bb211f590b914809556206b1622a0e7ff5f2 (patch) | |
tree | 5cb2d699e4bfeb85024dd32f8900eca5a62b51f8 | |
parent | acebbc103d958ff3418423042dea99b2a5325197 (diff) | |
download | cdo-1c67bb211f590b914809556206b1622a0e7ff5f2.tar.gz cdo-1c67bb211f590b914809556206b1622a0e7ff5f2.tar.xz cdo-1c67bb211f590b914809556206b1622a0e7ff5f2.zip |
[409284] Containment cycles can still occur drops/S20130528-1438
https://bugs.eclipse.org/bugs/show_bug.cgi?id=409284
12 files changed, 281 insertions, 293 deletions
diff --git a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CommitTransactionRequest.java b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CommitTransactionRequest.java index 5967a14172..3d071272a6 100644 --- a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CommitTransactionRequest.java +++ b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CommitTransactionRequest.java @@ -140,6 +140,7 @@ public class CommitTransactionRequest extends CDOClientRequestWithMonitoring<Com List<CDORevisionKey> changedObjects = commitData.getChangedObjects(); List<CDOIDAndVersion> detachedObjects = commitData.getDetachedObjects(); + out.writeLong(transaction.getLastUpdateTime()); out.writeBoolean(releaseLocks); out.writeString(comment); out.writeInt(newPackageUnits.size()); diff --git a/plugins/org.eclipse.emf.cdo.server.hibernate/src/org/eclipse/emf/cdo/server/internal/hibernate/HibernateRawCommitContext.java b/plugins/org.eclipse.emf.cdo.server.hibernate/src/org/eclipse/emf/cdo/server/internal/hibernate/HibernateRawCommitContext.java index c1aa8a0dd5..0a0e4f6cd4 100644 --- a/plugins/org.eclipse.emf.cdo.server.hibernate/src/org/eclipse/emf/cdo/server/internal/hibernate/HibernateRawCommitContext.java +++ b/plugins/org.eclipse.emf.cdo.server.hibernate/src/org/eclipse/emf/cdo/server/internal/hibernate/HibernateRawCommitContext.java @@ -102,6 +102,20 @@ public class HibernateRawCommitContext implements InternalCommitContext return null; } + public long getLastUpdateTime() + { + return 0; + } + + public long getTimeStamp() + { + return 0; + } + + public void setLastTreeRestructuringCommit(long lastTreeRestructuringCommit) + { + } + public boolean isAutoReleaseLocksEnabled() { return false; @@ -290,6 +304,10 @@ public class HibernateRawCommitContext implements InternalCommitContext { } + public void setLastUpdateTime(long lastUpdateTime) + { + } + public void setAutoReleaseLocksEnabled(boolean on) { } diff --git a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java index 495d485c74..5f214fb169 100644 --- a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java +++ b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/CommitTransactionIndication.java @@ -120,8 +120,8 @@ public class CommitTransactionIndication extends CDOServerIndicationWithMonitori initializeCommitContext(in); commitContext.preWrite(); + long lastUpdateTime = in.readLong(); boolean autoReleaseLocksEnabled = in.readBoolean(); - commitContext.setAutoReleaseLocksEnabled(autoReleaseLocksEnabled); String commitComment = in.readString(); InternalCDOPackageUnit[] newPackageUnits = new InternalCDOPackageUnit[in.readInt()]; @@ -266,6 +266,8 @@ public class CommitTransactionIndication extends CDOServerIndicationWithMonitori detachedObjectTypes = null; } + commitContext.setLastUpdateTime(lastUpdateTime); + commitContext.setAutoReleaseLocksEnabled(autoReleaseLocksEnabled); commitContext.setClearResourcePathCache(clearResourcePathCache); commitContext.setUsingEcore(usingEcore); commitContext.setUsingEtypes(usingEtypes); diff --git a/plugins/org.eclipse.emf.cdo.server/.settings/.api_filters b/plugins/org.eclipse.emf.cdo.server/.settings/.api_filters index edf8cf60cc..4a685f1204 100644 --- a/plugins/org.eclipse.emf.cdo.server/.settings/.api_filters +++ b/plugins/org.eclipse.emf.cdo.server/.settings/.api_filters @@ -88,30 +88,6 @@ </message_arguments> </filter> </resource> - <resource path="src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java" type="org.eclipse.emf.cdo.internal.server.TransactionCommitContext$DeltaLockWrapper"> - <filter id="571473929"> - <message_arguments> - <message_argument value="AbstractCDOID"/> - <message_argument value="DeltaLockWrapper"/> - </message_arguments> - </filter> - </resource> - <resource path="src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java" type="org.eclipse.emf.cdo.internal.server.TransactionCommitContext$DeltaLockWrapper$ForID"> - <filter id="574619656"> - <message_arguments> - <message_argument value="CDOID"/> - <message_argument value="ForID"/> - </message_arguments> - </filter> - </resource> - <resource path="src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java" type="org.eclipse.emf.cdo.internal.server.TransactionCommitContext$DeltaLockWrapper$ForIDAndBranch"> - <filter id="574619656"> - <message_arguments> - <message_argument value="CDOIDAndBranch"/> - <message_argument value="ForIDAndBranch"/> - </message_arguments> - </filter> - </resource> <resource path="src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java" type="org.eclipse.emf.cdo.internal.server.TransactionCommitContext$XRefContext"> <filter id="572522506"> <message_arguments> diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java index 7589ec06ef..4162dd6237 100644 --- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java +++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/Repository.java @@ -42,6 +42,8 @@ import org.eclipse.emf.cdo.common.revision.CDORevisionFactory; import org.eclipse.emf.cdo.common.revision.CDORevisionHandler; import org.eclipse.emf.cdo.common.revision.CDORevisionKey; import org.eclipse.emf.cdo.common.revision.CDORevisionUtil; +import org.eclipse.emf.cdo.common.revision.delta.CDOContainerFeatureDelta; +import org.eclipse.emf.cdo.common.revision.delta.CDOFeatureDelta; import org.eclipse.emf.cdo.common.util.CDOCommonUtil; import org.eclipse.emf.cdo.common.util.CDOQueryInfo; import org.eclipse.emf.cdo.common.util.RepositoryStateChangedEvent; @@ -74,6 +76,7 @@ import org.eclipse.emf.cdo.spi.common.model.InternalCDOPackageUnit; import org.eclipse.emf.cdo.spi.common.revision.DetachedCDORevision; import org.eclipse.emf.cdo.spi.common.revision.InternalCDOList; import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevision; +import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevisionDelta; import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevisionManager; import org.eclipse.emf.cdo.spi.common.revision.PointerCDORevision; import org.eclipse.emf.cdo.spi.common.revision.RevisionInfo; @@ -889,9 +892,20 @@ public class Repository extends Container<Object> implements InternalRepository timeStampAuthority.failCommit(timestamp); } + private long lastTreeRestructuringCommit = -1; + public void commit(InternalCommitContext commitContext, OMMonitor monitor) { - if (serializingCommits) + if (isTreeRestructuring(commitContext)) + { + synchronized (commitTransactionLock) + { + commitContext.setLastTreeRestructuringCommit(lastTreeRestructuringCommit); + commitUnsynced(commitContext, monitor); + lastTreeRestructuringCommit = commitContext.getTimeStamp(); + } + } + else if (serializingCommits) { synchronized (commitTransactionLock) { @@ -910,6 +924,23 @@ public class Repository extends Container<Object> implements InternalRepository distributor.run(InternalCommitContext.OPS, commitContext, monitor); } + private boolean isTreeRestructuring(InternalCommitContext commitContext) + { + for (InternalCDORevisionDelta delta : commitContext.getDirtyObjectDeltas()) + { + for (CDOFeatureDelta featureDelta : delta.getFeatureDeltas()) + { + EStructuralFeature feature = featureDelta.getFeature(); + if (feature == CDOContainerFeatureDelta.CONTAINER_FEATURE) + { + return true; + } + } + } + + return false; + } + @Deprecated public void sendCommitNotification(InternalSession sender, CDOCommitInfo commitInfo) { diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java index 75ef8a6d89..d654d79d16 100644 --- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java +++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java @@ -28,12 +28,10 @@ import org.eclipse.emf.cdo.common.lock.CDOLockOwner; import org.eclipse.emf.cdo.common.lock.CDOLockState; import org.eclipse.emf.cdo.common.lock.CDOLockUtil; import org.eclipse.emf.cdo.common.model.CDOPackageUnit; -import org.eclipse.emf.cdo.common.protocol.CDODataOutput; import org.eclipse.emf.cdo.common.revision.CDOIDAndBranch; import org.eclipse.emf.cdo.common.revision.CDOIDAndVersion; import org.eclipse.emf.cdo.common.revision.CDORevision; import org.eclipse.emf.cdo.common.revision.CDORevisionKey; -import org.eclipse.emf.cdo.common.revision.CDORevisionUtil; import org.eclipse.emf.cdo.common.revision.delta.CDOAddFeatureDelta; import org.eclipse.emf.cdo.common.revision.delta.CDOContainerFeatureDelta; import org.eclipse.emf.cdo.common.revision.delta.CDOFeatureDelta; @@ -53,7 +51,6 @@ import org.eclipse.emf.cdo.server.IStoreAccessor.QueryXRefsContext; import org.eclipse.emf.cdo.server.IView; import org.eclipse.emf.cdo.server.StoreThreadLocal; import org.eclipse.emf.cdo.spi.common.commit.InternalCDOCommitInfoManager; -import org.eclipse.emf.cdo.spi.common.id.AbstractCDOID; import org.eclipse.emf.cdo.spi.common.model.InternalCDOPackageInfo; import org.eclipse.emf.cdo.spi.common.model.InternalCDOPackageRegistry; import org.eclipse.emf.cdo.spi.common.model.InternalCDOPackageUnit; @@ -86,7 +83,6 @@ import org.eclipse.emf.ecore.EPackage; import org.eclipse.emf.ecore.EReference; import org.eclipse.emf.ecore.EStructuralFeature; -import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; @@ -123,6 +119,10 @@ public class TransactionCommitContext implements InternalCommitContext private IStoreAccessor accessor; + private long lastUpdateTime; + + private long lastTreeRestructuringCommit; + private long timeStamp = CDORevision.UNSPECIFIED_DATE; private long previousTimeStamp = CDORevision.UNSPECIFIED_DATE; @@ -212,6 +212,11 @@ public class TransactionCommitContext implements InternalCommitContext return commitComment; } + public long getLastUpdateTime() + { + return lastUpdateTime; + } + public boolean isAutoReleaseLocksEnabled() { return autoReleaseLocksEnabled; @@ -307,7 +312,7 @@ public class TransactionCommitContext implements InternalCommitContext return dirtyObjectDeltas; } - public CDORevision getRevision(CDOID id) + public InternalCDORevision getRevision(CDOID id) { if (cachedRevisions == null) { @@ -327,7 +332,7 @@ public class TransactionCommitContext implements InternalCommitContext } // Fall back to "before state" - return transaction.getRevision(id); + return (InternalCDORevision)transaction.getRevision(id); } private Map<CDOID, InternalCDORevision> cacheRevisions() @@ -423,6 +428,11 @@ public class TransactionCommitContext implements InternalCommitContext StoreThreadLocal.setCommitContext(this); } + public void setLastTreeRestructuringCommit(long lastTreeRestructuringCommit) + { + this.lastTreeRestructuringCommit = lastTreeRestructuringCommit; + } + public void setClearResourcePathCache(boolean clearResourcePathCache) { this.clearResourcePathCache = clearResourcePathCache; @@ -473,6 +483,11 @@ public class TransactionCommitContext implements InternalCommitContext this.detachedObjectVersions = detachedObjectVersions; } + public void setLastUpdateTime(long lastUpdateTime) + { + this.lastUpdateTime = lastUpdateTime; + } + public void setAutoReleaseLocksEnabled(boolean on) { autoReleaseLocksEnabled = on; @@ -553,6 +568,7 @@ public class TransactionCommitContext implements InternalCommitContext computeDirtyObjects(monitor.fork()); + checkContainmentCycles(); checkXRefs(); monitor.worked(); @@ -665,7 +681,7 @@ public class TransactionCommitContext implements InternalCommitContext return repository.createCommitTimeStamp(monitor); } - protected long getTimeStamp() + public long getTimeStamp() { return timeStamp; } @@ -854,23 +870,7 @@ public class TransactionCommitContext implements InternalCommitContext InternalCDORevisionDelta delta = dirtyObjectDeltas[i]; CDOID id = delta.getID(); Object key = lockManager.getLockKey(id, transaction.getBranch()); - if (serializingCommits) - { - lockedObjects.add(key); - } - else - { - lockedObjects.add(createDeltaLockWrapper(key, delta)); - - if (hasContainmentChanges(delta)) - { - if (isContainerLocked(delta)) - { - throw new ContainmentCycleDetectedException("Parent (" + key - + ") is already locked for containment changes"); - } - } - } + lockedObjects.add(key); } if (deltaTargetLocker != null) @@ -920,92 +920,6 @@ public class TransactionCommitContext implements InternalCommitContext } } - private DeltaLockWrapper createDeltaLockWrapper(Object key, InternalCDORevisionDelta delta) - { - if (key instanceof CDOID) - { - return new DeltaLockWrapper.ForID((CDOID)key, delta); - } - - if (key instanceof CDOIDAndBranch) - { - return new DeltaLockWrapper.ForIDAndBranch((CDOIDAndBranch)key, delta); - } - - throw new IllegalArgumentException("Invalid key: " + key); - } - - /** - * Iterates up the eContainers of an object and returns <code>true</code> on the first parent locked by another view. - * - * @return <code>true</code> if any parent is locked, <code>false</code> otherwise. - */ - private boolean isContainerLocked(InternalCDORevisionDelta delta) - { - CDOID id = delta.getID(); - InternalCDORevision revision = revisionManager.getRevisionByVersion(id, delta, CDORevision.UNCHUNKED, true); - if (revision == null) - { - // Can happen with non-auditing cache - throw new ConcurrentModificationException("Attempt by " + transaction + " to modify historical revision: " - + CDORevisionUtil.copyRevisionKey(delta)); - } - - return isContainerLocked(revision); - } - - private boolean isContainerLocked(InternalCDORevision revision) - { - CDOID id = (CDOID)revision.getContainerID(); - if (CDOIDUtil.isNull(id)) - { - return false; - } - - Object key = lockManager.getLockKey(id, transaction.getBranch()); - DeltaLockWrapper lockWrapper = createDeltaLockWrapper(key, null); - - if (lockManager.hasLockByOthers(LockType.WRITE, transaction, lockWrapper)) - { - Object object = lockManager.getLockEntryObject(lockWrapper); - if (object instanceof DeltaLockWrapper) - { - InternalCDORevisionDelta delta = ((DeltaLockWrapper)object).getDelta(); - if (delta != null && hasContainmentChanges(delta)) - { - return true; - } - } - } - - InternalCDORevision parent = revisionManager.getRevision(id, transaction, CDORevision.UNCHUNKED, - CDORevision.DEPTH_NONE, true); - - if (parent != null) - { - return isContainerLocked(parent); - } - - return false; - } - - private boolean hasContainmentChanges(InternalCDORevisionDelta delta) - { - for (CDOFeatureDelta featureDelta : delta.getFeatureDeltas()) - { - EStructuralFeature feature = featureDelta.getFeature(); - if (feature instanceof EReference) - { - if (((EReference)feature).isContainment()) - { - return true; - } - } - } - - return false; - } - private void lockTarget(Object value, Set<CDOID> newIDs, boolean supportingBranches) { if (value instanceof CDOIDObject) @@ -1054,6 +968,67 @@ public class TransactionCommitContext implements InternalCommitContext } } + protected void checkContainmentCycles() + { + if (lastTreeRestructuringCommit == 0) + { + // If this was a tree-restructuring commit then lastTreeRestructuringCommit would be initialized. + return; + } + + if (lastTreeRestructuringCommit <= lastUpdateTime) + { + // If this client's original state includes the state of the last tree-restructuring commit there's no danger. + return; + } + + Set<CDOID> objectsThatReachTheRoot = new HashSet<CDOID>(); + for (int i = 0; i < dirtyObjectDeltas.length; i++) + { + InternalCDORevisionDelta revisionDelta = dirtyObjectDeltas[i]; + CDOFeatureDelta containerDelta = revisionDelta.getFeatureDelta(CDOContainerFeatureDelta.CONTAINER_FEATURE); + if (containerDelta != null) + { + InternalCDORevision revision = dirtyObjects[i]; + if (!isTheRootReachable(revision, objectsThatReachTheRoot, new HashSet<CDOID>())) + { + throw new ContainmentCycleDetectedException("Attempt by " + transaction + " to introduce a containment cycle"); + } + } + } + } + + private boolean isTheRootReachable(InternalCDORevision revision, Set<CDOID> objectsThatReachTheRoot, + Set<CDOID> visited) + { + CDOID id = revision.getID(); + if (!visited.add(id)) + { + // Cycle detected on the way up to the root. + return false; + } + + if (!objectsThatReachTheRoot.add(id)) + { + // Has already been checked before. + return true; + } + + CDOID containerID = (CDOID)revision.getContainerID(); + if (CDOIDUtil.isNull(containerID)) + { + // The tree root has been reached. + return true; + } + + // Use this commit context as CDORevisionProvider for the container revisions. + // This is safe because Repository.commit() serializes all tree-restructuring commits. + InternalCDORevision containerRevision = getRevision(containerID); + + // Recurse Up + return isTheRootReachable(containerRevision, objectsThatReachTheRoot, visited); + } + private synchronized void unlockObjects() { if (!lockedObjects.isEmpty()) @@ -1455,157 +1430,6 @@ public class TransactionCommitContext implements InternalCommitContext } /** - * @author Martin Fluegge - */ - private static abstract class DeltaLockWrapper extends AbstractCDOID - { - private static final long serialVersionUID = 1L; - - private Object key; - - private InternalCDORevisionDelta delta; - - public DeltaLockWrapper(Object key, InternalCDORevisionDelta delta) - { - this.key = key; - this.delta = delta; - } - - public Object getKey() - { - return key; - } - - public InternalCDORevisionDelta getDelta() - { - return delta; - } - - public abstract CDOID getID(); - - @Override - public void write(CDODataOutput out) throws IOException - { - ((AbstractCDOID)getID()).write(out); - } - - public String toURIFragment() - { - return getID().toURIFragment(); - } - - public Type getType() - { - return getID().getType(); - } - - public boolean isObject() - { - return getID().isObject(); - } - - public boolean isTemporary() - { - return getID().isTemporary(); - } - - public boolean isExternal() - { - return getID().isExternal(); - } - - @Override - protected int doCompareTo(CDOID o) throws ClassCastException - { - return getID().compareTo(o); - } - - @Override - public boolean equals(Object obj) - { - if (obj instanceof DeltaLockWrapper) - { - DeltaLockWrapper wrapper = (DeltaLockWrapper)obj; - obj = wrapper.getKey(); - } - - if (key instanceof CDOID) - { - return key == obj; - } - - return key.equals(obj); - } - - @Override - public int hashCode() - { - return key.hashCode(); - } - - @Override - public String toString() - { - return key.toString(); - } - - /** - * @author Eike Stepper - */ - private static final class ForID extends DeltaLockWrapper - { - private static final long serialVersionUID = 1L; - - public ForID(CDOID key, InternalCDORevisionDelta delta) - { - super(key, delta); - } - - @Override - public CDOID getKey() - { - return (CDOID)super.getKey(); - } - - @Override - public CDOID getID() - { - return getKey(); - } - } - - /** - * @author Martin Fluegge - */ - private static final class ForIDAndBranch extends DeltaLockWrapper implements CDOIDAndBranch - { - private static final long serialVersionUID = 1L; - - public ForIDAndBranch(CDOIDAndBranch key, InternalCDORevisionDelta delta) - { - super(key, delta); - } - - @Override - public CDOIDAndBranch getKey() - { - return (CDOIDAndBranch)super.getKey(); - } - - @Override - public CDOID getID() - { - return getKey().getID(); - } - - public CDOBranch getBranch() - { - return getKey().getBranch(); - } - } - } - - /** * @author Eike Stepper */ private final class XRefContext implements QueryXRefsContext @@ -1653,7 +1477,6 @@ public class TransactionCommitContext implements InternalCommitContext { result.add(new CDOIDReference((CDOID)targetID, dirtyID[0], feature, index)); } - } return targetID; diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/syncing/ReplicatorCommitContext.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/syncing/ReplicatorCommitContext.java index 3bac2f7ae4..1c5ae8f161 100644 --- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/syncing/ReplicatorCommitContext.java +++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/syncing/ReplicatorCommitContext.java @@ -30,7 +30,7 @@ import java.util.List; /** * TODO Optimize createCommitInfo() - * + * * @author Eike Stepper */ public final class ReplicatorCommitContext extends TransactionCommitContext @@ -110,6 +110,12 @@ public final class ReplicatorCommitContext extends TransactionCommitContext // Do nothing } + @Override + protected void checkContainmentCycles() + { + // Do nothing + } + private static InternalCDOPackageUnit[] getNewPackageUnits(CDOCommitInfo commitInfo, InternalCDOPackageRegistry packageRegistry) { diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/ContainmentCycleDetectedException.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/ContainmentCycleDetectedException.java index f7d417e6d3..032d855b49 100644 --- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/ContainmentCycleDetectedException.java +++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/ContainmentCycleDetectedException.java @@ -10,10 +10,16 @@ */ package org.eclipse.emf.cdo.server; +import org.eclipse.emf.cdo.transaction.CDOTransaction; + /** - * An unchecked exception that is thrown from concurrent commit operations if cycles in the tree containmnent structure - * would result. - * + * An unchecked exception that can thrown from a commit operation that is based on stale information + * about the tree structure of the model and would introduce a containment cycle. + * <p> + * This situation results from a network race condition and can not be prevented by write locks on + * the changed objects. The committing client must {@link CDOTransaction#rollback() rollback} the transaction + * , replay the original changes and try to {@link CDOTransaction#commit() commit} again. + * * @author Eike Stepper * @since 4.0 */ diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/IStoreAccessor.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/IStoreAccessor.java index ccd628a09f..d09083d7b0 100644 --- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/IStoreAccessor.java +++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/server/IStoreAccessor.java @@ -307,6 +307,11 @@ public interface IStoreAccessor extends IQueryHandlerProvider, BranchLoader, Com public String getCommitComment(); /** + * @since 4.2 + */ + public long getLastUpdateTime(); + + /** * @since 3.0 */ public boolean isAutoReleaseLocksEnabled(); diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/InternalCommitContext.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/InternalCommitContext.java index bff8a6220c..fe37f0445e 100644 --- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/InternalCommitContext.java +++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/spi/server/InternalCommitContext.java @@ -64,6 +64,16 @@ public interface InternalCommitContext extends IStoreAccessor.CommitContext public InternalTransaction getTransaction(); + /** + * @since 4.2 + */ + public long getTimeStamp(); + + /** + * @since 4.2 + */ + public void setLastTreeRestructuringCommit(long lastTreeRestructuringCommit); + public void preWrite(); public void write(OMMonitor monitor); @@ -117,6 +127,11 @@ public interface InternalCommitContext extends IStoreAccessor.CommitContext */ public void setDetachedObjectVersions(CDOBranchVersion[] detachedObjectVersions); + /** + * @since 4.2 + */ + public void setLastUpdateTime(long lastUpdateTime); + public void setAutoReleaseLocksEnabled(boolean on); public void setCommitComment(String comment); diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_316444_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_316444_Test.java index 9a1d8ac8e1..dcaaff412a 100644 --- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_316444_Test.java +++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_316444_Test.java @@ -164,8 +164,11 @@ public class Bugzilla_316444_Test extends AbstractCDOTest map.put(RepositoryConfig.PROP_TEST_REPOSITORY, repository); } + /** + * Not needed anymore because of {@link Bugzilla_409284_Test}. + */ @CleanRepositoriesBefore - public void testMovingSubtree() throws Exception + public void _testMovingSubtree() throws Exception { exceptions.clear(); diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_409284_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_409284_Test.java new file mode 100644 index 0000000000..f09fbf2dea --- /dev/null +++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_409284_Test.java @@ -0,0 +1,102 @@ +/* + * Copyright (c) 2004 - 2012 Eike Stepper (Berlin, Germany) and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Eike Stepper - initial API and implementation + */ +package org.eclipse.emf.cdo.tests.bugzilla; + +import org.eclipse.emf.cdo.eresource.CDOResource; +import org.eclipse.emf.cdo.session.CDOSession; +import org.eclipse.emf.cdo.tests.AbstractCDOTest; +import org.eclipse.emf.cdo.tests.model1.Category; +import org.eclipse.emf.cdo.tests.model1.Company; +import org.eclipse.emf.cdo.transaction.CDOTransaction; +import org.eclipse.emf.cdo.util.CommitException; + +import org.eclipse.emf.common.util.EList; + +/** + * @author Eike Stepper + */ +public class Bugzilla_409284_Test extends AbstractCDOTest +{ + @SuppressWarnings("unused") + public void testContainmentCycle() throws Exception + { + // Client1 - Init + Category a1 = createCategory("A"); + Category b1 = createCategory("B"); + Category c1 = createCategory("C"); + Category d1 = createCategory("D"); + Category e1 = createCategory("E"); + + Company r1 = getModel1Factory().createCompany(); + r1.setName("R"); + + r1.getCategories().add(a1); + a1.getCategories().add(b1); + b1.getCategories().add(c1); + + r1.getCategories().add(d1); + d1.getCategories().add(e1); + + CDOSession session1 = openSession(); + CDOTransaction transaction1 = session1.openTransaction(); + CDOResource resource1 = transaction1.createResource(getResourcePath("res")); + + resource1.getContents().add(r1); + transaction1.commit(); + + // Client2 - Load + CDOSession session2 = openSession(); + session2.options().setPassiveUpdateEnabled(false); // Important! + CDOTransaction transaction2 = session2.openTransaction(); + CDOResource resource2 = transaction2.getResource(getResourcePath("res")); + + Company r2 = (Company)resource2.getContents().get(0); + + Category a2 = loadCategory(r2.getCategories(), 0, "A"); + Category b2 = loadCategory(a2.getCategories(), 0, "B"); + Category c2 = loadCategory(b2.getCategories(), 0, "C"); + + Category d2 = loadCategory(r2.getCategories(), 1, "D"); + Category e2 = loadCategory(d2.getCategories(), 0, "E"); + + // Client1 - First tree move (element B from A to E) + e1.getCategories().add(b1); + transaction1.commit(); + + // Client2 - Second tree move (element D from R to C) + c2.getCategories().add(d2); + + try + { + transaction2.commit(); + fail("CommitException expected"); + } + catch (CommitException expected) + { + String message = expected.getMessage(); + assertEquals(true, message.contains("ContainmentCycleDetectedException")); + } + } + + private Category createCategory(String name) + { + Category category = getModel1Factory().createCategory(); + category.setName(name); + return category; + } + + private Category loadCategory(EList<Category> categories, int i, String name) + { + Category category = categories.get(i); + assertEquals(name, category.getName()); + return category; + } +} |