Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMaxime Porhel2016-05-15 09:34:03 +0000
committerEike Stepper2016-05-15 09:34:03 +0000
commitbc0774025a6a7fbf129938e84a2d0b7ff7118d8f (patch)
tree332e322d0a53133a30bbae449ac43b62875c8e7c /plugins
parent45ead532133c5e2772a91b9d5075ca8e483d4ccf (diff)
downloadcdo-bc0774025a6a7fbf129938e84a2d0b7ff7118d8f.tar.gz
cdo-bc0774025a6a7fbf129938e84a2d0b7ff7118d8f.tar.xz
cdo-bc0774025a6a7fbf129938e84a2d0b7ff7118d8f.zip
[467174] Bad lock state with lock state and revision prefetch
https://bugs.eclipse.org/bugs/show_bug.cgi?id=467174 - Add Bugzilla_467174_Test JUnit test. - Add fix in CDOViewImpl and AbstractCDOView. Change-Id: Ic1ca47ac347a0909fd074bac53f222e57e25c9d8 Signed-off-by: Esteban Dugueperoux <esteban.dugueperoux@obeo.fr> Signed-off-by: Maxime Porhel <maxime.porhel@obeo.fr> Signed-off-by: Eike Stepper <stepper@esc-net.de>
Diffstat (limited to 'plugins')
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_467174_Test.java92
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java48
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java34
3 files changed, 149 insertions, 25 deletions
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_467174_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_467174_Test.java
new file mode 100644
index 0000000000..210bd72325
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_467174_Test.java
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2015 Obeo 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:
+ * Esteban Dugueperoux - initial API and implementation
+ */
+package org.eclipse.emf.cdo.tests.bugzilla;
+
+import org.eclipse.emf.cdo.CDOObject;
+import org.eclipse.emf.cdo.common.id.CDOID;
+import org.eclipse.emf.cdo.common.revision.CDORevision;
+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.CDOUtil;
+
+import org.eclipse.emf.internal.cdo.view.CDOViewImpl.OptionsImpl;
+
+import org.eclipse.net4j.util.concurrent.IRWLockManager.LockType;
+
+import org.eclipse.emf.common.notify.Notification;
+import org.eclipse.emf.common.notify.impl.AdapterImpl;
+import org.eclipse.emf.ecore.resource.ResourceSet;
+import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl;
+
+import org.junit.Assert;
+
+import java.util.Collections;
+
+/**
+ * Bug 467174 to test lock state and revision prefetch.
+ *
+ * @author Esteban Dugueperoux
+ */
+public class Bugzilla_467174_Test extends AbstractCDOTest
+{
+ public void testCDOObject_LockStateAndRevisionPrefetch() throws Exception
+ {
+ CDOSession session1 = openSession();
+ CDOTransaction transaction1 = session1.openTransaction();
+
+ OptionsImpl options = (OptionsImpl)transaction1.options();
+ options.setLockNotificationEnabled(true);
+ options.setLockStatePrefetchEnabled(true);
+
+ CDOResource resource1 = transaction1.createResource(getResourcePath("test1.model1"));
+ Company company = getModel1Factory().createCompany();
+ Category category = getModel1Factory().createCategory();
+ company.getCategories().add(category);
+ resource1.getContents().add(company);
+ resource1.save(Collections.emptyMap());
+
+ CDOObject companyCDOObject = CDOUtil.getCDOObject(company);
+ CDOID companyCDOID = companyCDOObject.cdoID();
+ CDOObject categoryCDOObject = CDOUtil.getCDOObject(category);
+ CDOID categoryCDOID = categoryCDOObject.cdoID();
+
+ CDOSession session2 = openSession();
+ ResourceSet resourceSet = new ResourceSetImpl();
+ resourceSet.eAdapters().add(new AdapterImpl()
+ {
+ @Override
+ public void notifyChanged(Notification msg)
+ {
+ Object newValue = msg.getNewValue();
+ if (newValue instanceof CDOResource)
+ {
+ ((CDOResource)newValue).cdoPrefetch(CDORevision.DEPTH_INFINITE);
+ }
+ }
+ });
+
+ CDOTransaction transaction2 = session2.openTransaction(resourceSet);
+ transaction2.options().setLockNotificationEnabled(true);
+ ((OptionsImpl)transaction2.options()).setLockStatePrefetchEnabled(true);
+
+ transaction1.lockObjects(Collections.singletonList(categoryCDOObject), LockType.WRITE, 1);
+ transaction1.lockObjects(Collections.singletonList(companyCDOObject), LockType.WRITE, 1);
+
+ CDOObject companyCDOObjectFromTx2 = transaction2.getObject(companyCDOID);
+ CDOObject categoryCDOObjectFromTx2 = transaction2.getObject(categoryCDOID);
+ Assert.assertTrue(companyCDOObjectFromTx2.cdoWriteLock().isLockedByOthers());
+ Assert.assertTrue(categoryCDOObjectFromTx2.cdoWriteLock().isLockedByOthers());
+ }
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java
index fbbfe8da88..64b46d5dff 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java
@@ -167,6 +167,8 @@ public abstract class AbstractCDOView extends CDOCommitHistoryProviderImpl<CDOOb
private Map<CDOID, InternalCDOObject> objects;
+ private int objectCreationCounter;
+
private CDOStore store = new CDOStoreImpl(this);
private CDOResourceImpl rootResource;
@@ -1824,32 +1826,44 @@ public abstract class AbstractCDOView extends CDOCommitHistoryProviderImpl<CDOOb
TRACER.trace("Creating object for " + id); //$NON-NLS-1$
}
+ // Remember the current object creation count to be able to optimize the map lookup
+ // away if no objects get created during the getRevision() call.
+ int originalCount = objectCreationCounter;
+
InternalCDORevision revision = getRevision(id, true);
if (revision == null)
{
throw new ObjectNotFoundException(id, this);
}
- EClass eClass = revision.getEClass();
- InternalCDOObject object;
- if (CDOModelUtil.isResource(eClass) && id != rootResourceID)
- {
- object = (InternalCDOObject)newResourceInstance(revision);
- // object is PROXY
- }
- else
+ // If the object has been created and registered during revision loading
+ // (for example by lock state prefetcher) don't create a duplicate.
+ InternalCDOObject object = objectCreationCounter == originalCount ? null : objects.get(id);
+ if (object == null)
{
- object = newInstance(eClass);
- // object is TRANSIENT
- }
+ EClass eClass = revision.getEClass();
+
+ if (CDOModelUtil.isResource(eClass) && id != rootResourceID)
+ {
+ object = (InternalCDOObject)newResourceInstance(revision);
+ // object is PROXY
+ }
+ else
+ {
+ object = newInstance(eClass);
+ // object is TRANSIENT
+ }
- cleanObject(object, revision);
- CDOStateMachine.INSTANCE.dispatchLoadNotification(object);
+ ++objectCreationCounter;
- // Bug 435198: Have object's resource added to the ResourceSet on call to CDOView.getObject(CDOID)
- if (!CDOModelUtil.isResource(eClass))
- {
- getStore().getResource(object);
+ cleanObject(object, revision);
+ CDOStateMachine.INSTANCE.dispatchLoadNotification(object);
+
+ // Bug 435198: Have object's resource added to the ResourceSet on call to CDOView.getObject(CDOID)
+ if (!CDOModelUtil.isResource(eClass))
+ {
+ getStore().getResource(object);
+ }
}
return object;
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java
index 71a6e6c5cd..694efefc37 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java
@@ -455,6 +455,18 @@ public class CDOViewImpl extends AbstractCDOView implements IExecutorServiceProv
*/
protected void updateLockStates(CDOLockState[] newLockStates)
{
+ updateLockStates(newLockStates, false);
+ }
+
+ /**
+ * Updates the lock states of objects held in this view
+ *
+ * @param newLockStates new {@link CDOLockState lockStates} to integrate in cache
+ * @param loadOnDemand true to load corresponding {@link CDOObject} if not already loaded to be able to store lockState in cache, false otherwise
+ * @since 4.5
+ */
+ protected void updateLockStates(CDOLockState[] newLockStates, boolean loadOnDemand)
+ {
for (CDOLockState lockState : newLockStates)
{
Object lockedObject = lockState.getLockedObject();
@@ -478,7 +490,7 @@ public class CDOViewImpl extends AbstractCDOView implements IExecutorServiceProv
throw new IllegalStateException("Unexpected: " + lockedObject.getClass().getSimpleName());
}
- InternalCDOObject object = getObject(id, false);
+ InternalCDOObject object = getObject(id, loadOnDemand);
if (object != null)
{
InternalCDOLockState existingLockState = (InternalCDOLockState)lockStates.get(object);
@@ -2660,7 +2672,7 @@ public class CDOViewImpl extends AbstractCDOView implements IExecutorServiceProv
CDOSessionProtocol sessionProtocol = session.getSessionProtocol();
CDOLockState[] lockStates = sessionProtocol.getLockStates(viewID, ids, revisionsLoadedEvent.getPrefetchDepth());
- updateLockStatesForAllViews(lockStates);
+ updateLockStatesForAllViews(lockStates, true);
// add missing lock states
List<CDOLockState> missingLockStates = new ArrayList<CDOLockState>();
@@ -2671,7 +2683,10 @@ public class CDOViewImpl extends AbstractCDOView implements IExecutorServiceProv
if (object != null && CDOViewImpl.this.lockStates.get(object) == null)
{
Object lockedObject = getLockTarget(object); // CDOID or CDOIDAndBranch
- missingLockStates.add(CDOLockUtil.createLockState(lockedObject));
+ if (lockedObject != null)
+ {
+ missingLockStates.add(CDOLockUtil.createLockState(lockedObject));
+ }
}
}
@@ -2685,24 +2700,27 @@ public class CDOViewImpl extends AbstractCDOView implements IExecutorServiceProv
if (object != null && CDOViewImpl.this.lockStates.get(object) == null)
{
Object lockedObject = getLockTarget(object); // CDOID or CDOIDAndBranch
- missingLockStates.add(CDOLockUtil.createLockState(lockedObject));
+ if (lockedObject != null)
+ {
+ missingLockStates.add(CDOLockUtil.createLockState(lockedObject));
+ }
}
}
}
- updateLockStatesForAllViews(missingLockStates.toArray(new CDOLockState[missingLockStates.size()]));
+ updateLockStatesForAllViews(missingLockStates.toArray(new CDOLockState[missingLockStates.size()]), false);
}
}
- private void updateLockStatesForAllViews(CDOLockState[] lockStates)
+ private void updateLockStatesForAllViews(CDOLockState[] lockStates, boolean loadOnDemand)
{
- updateLockStates(lockStates);
+ updateLockStates(lockStates, loadOnDemand);
for (CDOCommonView view : getSession().getViews())
{
if (view != CDOViewImpl.this && view.getBranch() == getBranch())
{
- updateLockStates(lockStates);
+ updateLockStates(lockStates, loadOnDemand);
}
}
}

Back to the top