diff --git a/source/java/org/alfresco/repo/domain/locks/AbstractLockDAOImpl.java b/source/java/org/alfresco/repo/domain/locks/AbstractLockDAOImpl.java index e40e90c1b2..4eb8d0548b 100644 --- a/source/java/org/alfresco/repo/domain/locks/AbstractLockDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/locks/AbstractLockDAOImpl.java @@ -175,22 +175,6 @@ public abstract class AbstractLockDAOImpl implements LockDAO return updateLocks(lockQName, lockToken, LOCK_TOKEN_RELEASED, 0L, optimistic); } - @Override - public boolean releaseLockQuiet(QName lockQName, String lockToken) - { - try - { - updateLocks(lockQName, lockToken, LOCK_TOKEN_RELEASED, 0L, false); - // It worked - return true; - } - catch (LockAcquisitionException e) - { - // We absorb this - return false; - } - } - /** * Put new values against the given exclusive lock. This works against the related locks as well. * @param optimistic true if a mismatch in the number of locked rows should diff --git a/source/java/org/alfresco/repo/domain/locks/LockDAO.java b/source/java/org/alfresco/repo/domain/locks/LockDAO.java index a386728093..c07bbc1b48 100644 --- a/source/java/org/alfresco/repo/domain/locks/LockDAO.java +++ b/source/java/org/alfresco/repo/domain/locks/LockDAO.java @@ -30,7 +30,7 @@ import org.alfresco.service.namespace.QName; public interface LockDAO { /** - * Aquire a given exclusive lock, assigning it (and any implicitly shared locks) a + * Acquire a given exclusive lock, assigning it (and any implicitly shared locks) a * timeout. All shared locks are implicitly taken as well. *

* A lock can be re-taken if it has expired and if the lock token has not changed @@ -78,15 +78,4 @@ public interface LockDAO * and pessimistic release is requested. */ boolean releaseLock(QName lockQName, String lockToken, boolean optimistic); - - /** - * Release a lock without throwing any exceptions if the lock was not updated. - * - * @param lockQName the unique name of the lock to release - * @param lockToken the current lock token - * @return Returns true if all the required locks were - * (still) held under the lock token and were - * valid at the time of release, otherwise false - */ - boolean releaseLockQuiet(QName lockQName, String lockToken); } diff --git a/source/java/org/alfresco/repo/lock/JobLockService.java b/source/java/org/alfresco/repo/lock/JobLockService.java index bed6c43d9b..ca44345e9f 100644 --- a/source/java/org/alfresco/repo/lock/JobLockService.java +++ b/source/java/org/alfresco/repo/lock/JobLockService.java @@ -169,27 +169,23 @@ public interface JobLockService void refreshLock(String lockToken, QName lockQName, long timeToLive, JobLockRefreshCallback callback); /** - * Release the lock using a valid lock token. The lock can have expired or even been taken - * by another processes (i.e. the lock token will no longer be valid); none of this will - * prevent the method from succeeding. This operation is functionally the same as the newer - * {@link #releaseLockVerify(String, QName)} operation, other than it returns void. Retained - * for backwards-compatibility. + * Release the lock using a valid lock token. * * @param lockToken the lock token returned when the lock was acquired * @param lockQName the name of the previously-acquired lock + * @throws LockAcquisitionException if the lock has been taken over by another process */ void releaseLock(String lockToken, QName lockQName); /** - * Release the lock using a valid lock token. The lock can have expired or even been taken - * by another processes (i.e. the lock token will no longer be valid); none of this will - * prevent the method from succeeding. Functionally similar to {@link #releaseLock(String, QName)}, but - * this newer operation indicates whether a lock was actually released by its return value. + * Release the lock using a valid lock token. The lock can have been taken + * by another process (i.e. the lock token will no longer be valid); none of this will + * prevent the method from succeeding. * * @param lockToken the lock token returned when the lock was acquired * @param lockQName the name of the previously-acquired lock * @return true if the lock was valid and released otherwise - * false if the lock was no longer valid in any case + * false if the lock was already held by another token */ boolean releaseLockVerify(String lockToken, QName lockQName); diff --git a/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java b/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java index 796c8d968d..c3c16b7c03 100644 --- a/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java +++ b/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java @@ -354,7 +354,7 @@ public class JobLockServiceImpl implements JobLockService // Release the lock in case the initiator did not do it. // We just want to release and don't care if the lock was already released // or taken by another process - if (releaseLockQuiet(lockToken, lockQName)) + if (releaseLockVerify(lockToken, lockQName)) { // The callback must be informed as we released the lock automatically callLockReleased(callback); @@ -424,7 +424,14 @@ public class JobLockServiceImpl implements JobLockService @Override public void releaseLock(final String lockToken, final QName lockQName) { - releaseLockVerify(lockToken, lockQName); + RetryingTransactionCallback releaseCallback = new RetryingTransactionCallback() + { + public Boolean execute() throws Throwable + { + return lockDAO.releaseLock(lockQName, lockToken, false); + } + }; + retryingTransactionHelper.doInTransaction(releaseCallback, false, true); } /** @@ -442,27 +449,6 @@ public class JobLockServiceImpl implements JobLockService return retryingTransactionHelper.doInTransaction(releaseCallback, false, true); } - /** - * Attempt to release a lock but do not worry about not being able to update the lock. - * If the lock was taken by another process, then it will not matter. Any other database-related - * conditions will still trigger a retry. - * - * @param lockToken the unique lock token to release (expired or not) - * @param lockQName the name of the lock - * @return true if the lock was released or false if not - */ - private boolean releaseLockQuiet(final String lockToken, final QName lockQName) - { - RetryingTransactionCallback releaseCallback = new RetryingTransactionCallback() - { - public Boolean execute() throws Throwable - { - return lockDAO.releaseLockQuiet(lockQName, lockToken); - } - }; - return retryingTransactionHelper.doInTransaction(releaseCallback, false, true); - } - /** * @throws LockAcquisitionException on failure */ diff --git a/source/test-java/org/alfresco/repo/domain/locks/LockDAOTest.java b/source/test-java/org/alfresco/repo/domain/locks/LockDAOTest.java index fa0a7c9590..e58f6c7bb9 100644 --- a/source/test-java/org/alfresco/repo/domain/locks/LockDAOTest.java +++ b/source/test-java/org/alfresco/repo/domain/locks/LockDAOTest.java @@ -238,12 +238,12 @@ public class LockDAOTest extends TestCase String token = lock(lockAAA, 500000L, true); release(lockAAA, token, true); token = lock(lockAAA, 0L, true); - - // Check that the lock cannot be release when not held - release(lockAAA, "Invalid-Token", false); - assertFalse(lockDAO.releaseLockQuiet(lockAAA, "invalidToken")); - assertTrue(lockDAO.releaseLockQuiet(lockAAA, token)); - assertFalse(lockDAO.releaseLockQuiet(lockAAA, token)); + + // Check that the lock cannot be release when not held + release(lockAAA, "Invalid-Token", false); + assertFalse(lockDAO.releaseLock(lockAAA, "invalidToken", true)); + assertTrue(lockDAO.releaseLock(lockAAA, token, true)); + assertFalse(lockDAO.releaseLock(lockAAA, token, true)); } public void testReleaseLockRepeated() throws Exception diff --git a/source/test-java/org/alfresco/repo/lock/JobLockServiceTest.java b/source/test-java/org/alfresco/repo/lock/JobLockServiceTest.java index 852d089359..752edc3add 100644 --- a/source/test-java/org/alfresco/repo/lock/JobLockServiceTest.java +++ b/source/test-java/org/alfresco/repo/lock/JobLockServiceTest.java @@ -380,7 +380,7 @@ public class JobLockServiceTest extends TestCase // The first refresh will occur in 500ms wait(1000L); // Should NOT get a callback saying that the lock has been released - assertTrue("Lock should be optimistically released", released[0] > 0); + assertFalse("Lock should be optimistically released", released[0] > 0); try { jobLockService.getLock(lockQName, lockTTL);