summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorCaspar De Groot2011-02-11 04:46:19 (EST)
committerCaspar De Groot2011-02-11 04:46:19 (EST)
commit140b526c1b0ffd91d9504acbe0f5780b367727ab (patch)
tree7192f7f8be5b4584ea9789bc57d41c3d91e77d58
parent32a7380da14ed80145d4c4ee75691b03802fb076 (diff)
downloadcdo-140b526c1b0ffd91d9504acbe0f5780b367727ab.zip
cdo-140b526c1b0ffd91d9504acbe0f5780b367727ab.tar.gz
cdo-140b526c1b0ffd91d9504acbe0f5780b367727ab.tar.bz2
[Bug 334981] LockObjectsIndication inappropriately throws exception for stale revisions
https://bugs.eclipse.org/bugs/show_bug.cgi?id=334981
-rw-r--r--plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CDOClientProtocol.java3
-rw-r--r--plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/LockObjectsRequest.java32
-rw-r--r--plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LockObjectsIndication.java46
-rw-r--r--plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/embedded/EmbeddedClientSessionProtocol.java3
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/LockingManagerTest.java11
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/LockTimeoutException.java26
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/StaleRevisionLockException.java38
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLockImpl.java6
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/DelegatingSessionProtocol.java3
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java6
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/CDOSessionProtocol.java3
11 files changed, 152 insertions, 25 deletions
diff --git a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CDOClientProtocol.java b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CDOClientProtocol.java
index 3fd2fc4..879869b 100644
--- a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CDOClientProtocol.java
+++ b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/CDOClientProtocol.java
@@ -27,6 +27,7 @@ import org.eclipse.emf.cdo.common.lob.CDOLobInfo;
import org.eclipse.emf.cdo.common.model.CDOPackageUnit;
import org.eclipse.emf.cdo.common.protocol.CDOProtocolConstants;
import org.eclipse.emf.cdo.common.revision.CDORevisionHandler;
+import org.eclipse.emf.cdo.common.util.CDOException;
import org.eclipse.emf.cdo.common.util.TransportException;
import org.eclipse.emf.cdo.internal.net4j.bundle.OM;
import org.eclipse.emf.cdo.session.CDOSession;
@@ -210,7 +211,7 @@ public class CDOClientProtocol extends SignalProtocol<CDOSession> implements CDO
}
}
- public boolean lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
+ public CDOException lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
LockType lockType, long timeout) throws InterruptedException
{
InterruptedException interruptedException = null;
diff --git a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/LockObjectsRequest.java b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/LockObjectsRequest.java
index 2c3cc86..6fb2739 100644
--- a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/LockObjectsRequest.java
+++ b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/protocol/LockObjectsRequest.java
@@ -7,6 +7,7 @@
*
* Contributors:
* Eike Stepper - initial API and implementation
+ * Caspar De Groot - maintenance
**************************************************************************/
package org.eclipse.emf.cdo.internal.net4j.protocol;
@@ -15,7 +16,11 @@ import org.eclipse.emf.cdo.common.protocol.CDODataInput;
import org.eclipse.emf.cdo.common.protocol.CDODataOutput;
import org.eclipse.emf.cdo.common.protocol.CDOProtocolConstants;
import org.eclipse.emf.cdo.common.revision.CDORevision;
+import org.eclipse.emf.cdo.common.revision.CDORevisionKey;
+import org.eclipse.emf.cdo.common.util.CDOException;
import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevision;
+import org.eclipse.emf.cdo.util.LockTimeoutException;
+import org.eclipse.emf.cdo.util.StaleRevisionLockException;
import org.eclipse.net4j.util.concurrent.IRWLockManager;
import org.eclipse.net4j.util.concurrent.IRWLockManager.LockType;
@@ -24,9 +29,9 @@ import java.io.IOException;
import java.util.List;
/**
- * @author Eike Stepper
+ * @author Eike Stepper, Caspar De Groot
*/
-public class LockObjectsRequest extends CDOClientRequest<Boolean>
+public class LockObjectsRequest extends CDOClientRequest<CDOException>
{
private int viewID;
@@ -69,8 +74,27 @@ public class LockObjectsRequest extends CDOClientRequest<Boolean>
}
@Override
- protected Boolean confirming(CDODataInput in) throws IOException
+ protected CDOException confirming(CDODataInput in) throws IOException
{
- return in.readBoolean();
+ boolean success = in.readBoolean();
+ if (success)
+ {
+ return null;
+ }
+
+ boolean timedOut = in.readBoolean();
+ if (timedOut)
+ {
+ return new LockTimeoutException();
+ }
+
+ int nStaleRevisions = in.readInt();
+ CDORevisionKey[] staleRevisions = new CDORevisionKey[nStaleRevisions];
+ for (int i = 0; i < nStaleRevisions; i++)
+ {
+ staleRevisions[i] = in.readCDORevisionKey();
+ }
+
+ return new StaleRevisionLockException(staleRevisions);
}
}
diff --git a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LockObjectsIndication.java b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LockObjectsIndication.java
index 31dfcef..c060960 100644
--- a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LockObjectsIndication.java
+++ b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LockObjectsIndication.java
@@ -26,9 +26,11 @@ import org.eclipse.emf.cdo.spi.server.InternalLockManager;
import org.eclipse.net4j.util.WrappedException;
import org.eclipse.net4j.util.concurrent.IRWLockManager.LockType;
+import org.eclipse.net4j.util.concurrent.TimeoutRuntimeException;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.LinkedList;
import java.util.List;
/**
@@ -38,6 +40,10 @@ public class LockObjectsIndication extends CDOReadIndication
{
private List<Object> objectsToBeLocked = new ArrayList<Object>();
+ private List<CDORevisionKey> staleRevisions = new LinkedList<CDORevisionKey>();
+
+ private boolean timedOut;
+
public LockObjectsIndication(CDOServerProtocol protocol)
{
super(protocol, CDOProtocolConstants.SIGNAL_LOCK_OBJECTS);
@@ -59,12 +65,22 @@ public class LockObjectsIndication extends CDOReadIndication
handleViewedRevision(viewedBranch, revKeys[i]);
}
+ if (staleRevisions.size() > 0)
+ {
+ // If we have 1 or more stale revisions, we should not lock
+ return;
+ }
+
IView view = getSession().getView(viewID);
InternalLockManager lockManager = getRepository().getLockManager();
try
{
lockManager.lock(lockType, view, objectsToBeLocked, timeout);
}
+ catch (TimeoutRuntimeException ex)
+ {
+ timedOut = true;
+ }
catch (InterruptedException ex)
{
throw WrappedException.wrap(ex);
@@ -73,23 +89,23 @@ public class LockObjectsIndication extends CDOReadIndication
private void handleViewedRevision(CDOBranch viewedBranch, CDORevisionKey revKey)
{
- // TODO (CD) I'm using IllegalArgExceptions here because that's how it worked before. But
- // personally I don't think it makes a lot of sense to throw exceptions from #indicating or
- // #responding when *expected* problems are detected, especially because they trigger
- // a separate signal. Why not just report problems through the response stream?
-
CDOID id = revKey.getID();
InternalCDORevision rev = getRepository().getRevisionManager().getRevision(id, viewedBranch.getHead(),
CDORevision.UNCHUNKED, CDORevision.DEPTH_NONE, true);
+
if (rev == null)
{
throw new IllegalArgumentException(String.format("Object %s not found in branch %s (possibly detached)", id,
viewedBranch));
}
+
if (!revKey.equals(rev))
{
- throw new IllegalArgumentException(String.format(
- "Client's revision of object %s is not the latest version in branch %s", id, viewedBranch));
+ staleRevisions.add(revKey);
+
+ // If we have 1 or more stale revisions, the locking won't proceed for sure,
+ // so we can return early
+ return;
}
if (getRepository().isSupportingBranches())
@@ -105,6 +121,20 @@ public class LockObjectsIndication extends CDOReadIndication
@Override
protected void responding(CDODataOutput out) throws IOException
{
- out.writeBoolean(true);
+ boolean success = !timedOut && staleRevisions.size() == 0;
+ out.writeBoolean(success);
+
+ if (!success)
+ {
+ out.writeBoolean(timedOut);
+ if (!timedOut)
+ {
+ out.writeInt(staleRevisions.size());
+ for (CDORevisionKey staleRevision : staleRevisions)
+ {
+ out.writeCDORevisionKey(staleRevision);
+ }
+ }
+ }
}
}
diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/embedded/EmbeddedClientSessionProtocol.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/embedded/EmbeddedClientSessionProtocol.java
index b57fd3e..0471e63 100644
--- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/embedded/EmbeddedClientSessionProtocol.java
+++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/embedded/EmbeddedClientSessionProtocol.java
@@ -30,6 +30,7 @@ import org.eclipse.emf.cdo.common.protocol.CDOAuthenticator;
import org.eclipse.emf.cdo.common.revision.CDOIDAndVersion;
import org.eclipse.emf.cdo.common.revision.CDORevisionHandler;
import org.eclipse.emf.cdo.common.revision.CDORevisionKey;
+import org.eclipse.emf.cdo.common.util.CDOException;
import org.eclipse.emf.cdo.common.util.CDOQueryQueue;
import org.eclipse.emf.cdo.server.StoreThreadLocal;
import org.eclipse.emf.cdo.session.remote.CDORemoteSession;
@@ -324,7 +325,7 @@ public class EmbeddedClientSessionProtocol extends Lifecycle implements CDOSessi
throw new UnsupportedOperationException();
}
- public boolean lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
+ public CDOException lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
LockType lockType, long timeout) throws InterruptedException
{
throw new UnsupportedOperationException();
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/LockingManagerTest.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/LockingManagerTest.java
index 44e7d0e..ab5de8e 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/LockingManagerTest.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/LockingManagerTest.java
@@ -21,10 +21,11 @@ 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.cdo.util.CommitException;
+import org.eclipse.emf.cdo.util.StaleRevisionLockException;
+import org.eclipse.emf.cdo.util.LockTimeoutException;
import org.eclipse.net4j.util.concurrent.IRWLockManager.LockType;
import org.eclipse.net4j.util.concurrent.RWLockManager;
-import org.eclipse.net4j.util.concurrent.TimeoutRuntimeException;
import org.eclipse.net4j.util.io.IOUtil;
import java.util.Collections;
@@ -259,9 +260,9 @@ public class LockingManagerTest extends AbstractCDOTest
try
{
transaction2.lockObjects(Collections.singletonList(cdoCompany2), LockType.WRITE, 1000);
- fail("Should have an exception");
+ fail("Should have thrown an exception");
}
- catch (TimeoutRuntimeException ex)
+ catch (LockTimeoutException ex)
{
}
@@ -333,7 +334,7 @@ public class LockingManagerTest extends AbstractCDOTest
transaction2.lockObjects(Collections.singletonList(cdoCompany2), LockType.WRITE, 1000);
fail("Should have an exception");
}
- catch (TimeoutRuntimeException ex)
+ catch (LockTimeoutException ex)
{
}
}
@@ -741,7 +742,7 @@ public class LockingManagerTest extends AbstractCDOTest
}
fail("Should have thrown IllegalArgumentException");
}
- catch (IllegalArgumentException e)
+ catch (StaleRevisionLockException e)
{
}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/LockTimeoutException.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/LockTimeoutException.java
new file mode 100644
index 0000000..4080d3d
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/LockTimeoutException.java
@@ -0,0 +1,26 @@
+/**
+ * Copyright (c) 2004 - 2011 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:
+ * Caspar De Groot - initial API and implementation
+ */
+package org.eclipse.emf.cdo.util;
+
+import org.eclipse.emf.cdo.common.util.CDOException;
+
+/**
+ * @author Caspar De Groot
+ * @since 4.0
+ */
+public class LockTimeoutException extends CDOException
+{
+ private static final long serialVersionUID = 293135415513673577L;
+
+ public LockTimeoutException()
+ {
+ }
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/StaleRevisionLockException.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/StaleRevisionLockException.java
new file mode 100644
index 0000000..f318d1c
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/util/StaleRevisionLockException.java
@@ -0,0 +1,38 @@
+/**
+ * Copyright (c) 2004 - 2011 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:
+ * Caspar De Groot - initial API and implementation
+ */
+package org.eclipse.emf.cdo.util;
+
+import org.eclipse.emf.cdo.common.revision.CDORevisionKey;
+import org.eclipse.emf.cdo.common.util.CDOException;
+
+import org.eclipse.net4j.util.CheckUtil;
+
+/**
+ * @author Caspar De Groot
+ * @since 4.0
+ */
+public class StaleRevisionLockException extends CDOException
+{
+ private static final long serialVersionUID = 5821185370877023119L;
+
+ private final CDORevisionKey[] staleRevisions;
+
+ public StaleRevisionLockException(CDORevisionKey[] staleRevisions)
+ {
+ CheckUtil.checkArg(staleRevisions, "staleRevisions");
+ this.staleRevisions = staleRevisions;
+ }
+
+ public CDORevisionKey[] getStaleRevisions()
+ {
+ return staleRevisions;
+ }
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLockImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLockImpl.java
index 0af1999..94558a8 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLockImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLockImpl.java
@@ -12,10 +12,10 @@
package org.eclipse.emf.internal.cdo.object;
import org.eclipse.emf.cdo.CDOLock;
+import org.eclipse.emf.cdo.util.LockTimeoutException;
import org.eclipse.net4j.util.WrappedException;
import org.eclipse.net4j.util.concurrent.IRWLockManager.LockType;
-import org.eclipse.net4j.util.concurrent.TimeoutRuntimeException;
import org.eclipse.emf.spi.cdo.InternalCDOObject;
@@ -115,7 +115,7 @@ public class CDOLockImpl implements CDOLock
object.cdoView().lockObjects(Collections.singletonList(object), type, CDOLock.NO_WAIT);
return true;
}
- catch (TimeoutRuntimeException ex)
+ catch (LockTimeoutException ex)
{
return false;
}
@@ -132,7 +132,7 @@ public class CDOLockImpl implements CDOLock
object.cdoView().lockObjects(Collections.singletonList(object), type, unit.toMillis(time));
return true;
}
- catch (TimeoutRuntimeException ex)
+ catch (LockTimeoutException ex)
{
return false;
}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/DelegatingSessionProtocol.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/DelegatingSessionProtocol.java
index b56a0ce..a0c485b 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/DelegatingSessionProtocol.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/DelegatingSessionProtocol.java
@@ -26,6 +26,7 @@ import org.eclipse.emf.cdo.common.lob.CDOLob;
import org.eclipse.emf.cdo.common.lob.CDOLobInfo;
import org.eclipse.emf.cdo.common.model.CDOPackageUnit;
import org.eclipse.emf.cdo.common.revision.CDORevisionHandler;
+import org.eclipse.emf.cdo.common.util.CDOException;
import org.eclipse.emf.cdo.session.CDOSession;
import org.eclipse.emf.cdo.session.CDOSession.ExceptionHandler;
import org.eclipse.emf.cdo.session.remote.CDORemoteSession;
@@ -544,7 +545,7 @@ public class DelegatingSessionProtocol extends Lifecycle implements CDOSessionPr
}
}
- public boolean lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
+ public CDOException lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
LockType lockType, long timeout) throws InterruptedException
{
int attempt = 0;
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 854b84b..910d779 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
@@ -248,7 +248,11 @@ public class CDOViewImpl extends AbstractCDOView
}
CDOSessionProtocol sessionProtocol = session.getSessionProtocol();
- sessionProtocol.lockObjects(revisions, viewID, getBranch(), lockType, timeout);
+ CDOException exception = sessionProtocol.lockObjects(revisions, viewID, getBranch(), lockType, timeout);
+ if (exception != null)
+ {
+ throw exception;
+ }
}
/**
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/CDOSessionProtocol.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/CDOSessionProtocol.java
index 76e8e64..b70d8db 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/CDOSessionProtocol.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/CDOSessionProtocol.java
@@ -30,6 +30,7 @@ import org.eclipse.emf.cdo.common.revision.CDOReferenceAdjuster;
import org.eclipse.emf.cdo.common.revision.CDORevisionHandler;
import org.eclipse.emf.cdo.common.revision.CDORevisionKey;
import org.eclipse.emf.cdo.common.util.CDOCommonUtil;
+import org.eclipse.emf.cdo.common.util.CDOException;
import org.eclipse.emf.cdo.session.remote.CDORemoteSession;
import org.eclipse.emf.cdo.session.remote.CDORemoteSessionMessage;
import org.eclipse.emf.cdo.spi.common.CDORawReplicationContext;
@@ -125,7 +126,7 @@ public interface CDOSessionProtocol extends CDOProtocol, PackageLoader, BranchLo
/**
* @since 4.0
*/
- public boolean lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
+ public CDOException lockObjects(List<InternalCDORevision> viewedRevisions, int viewID, CDOBranch viewedBranch,
LockType lockType, long timeout) throws InterruptedException;
/**