From 517569d27b8c011c1a1b68ed0525fa73c08ee127 Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Thu, 7 Oct 2010 14:54:27 +0000 Subject: [PATCH] Fixed JobLockService refresh callback code - Fallout from ALF-4898 - Unit tests perform and check callback counts git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@22966 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../alfresco/repo/lock/JobLockService.java | 2 +- .../repo/lock/JobLockServiceImpl.java | 8 ++- .../repo/lock/JobLockServiceTest.java | 69 +++++++++++++++---- 3 files changed, 60 insertions(+), 19 deletions(-) diff --git a/source/java/org/alfresco/repo/lock/JobLockService.java b/source/java/org/alfresco/repo/lock/JobLockService.java index bc158a11a7..2f264ec21f 100644 --- a/source/java/org/alfresco/repo/lock/JobLockService.java +++ b/source/java/org/alfresco/repo/lock/JobLockService.java @@ -228,7 +228,7 @@ public interface JobLockService * to terminate. *

* This method is also called if the initiating process is self-terminated i.e. if the originating - * process releases the lock itself. This method is not called if the process is not + * process releases the lock itself. This method is not called if the process is not * {@link #isActive() active}. * * @since 3.4.0b diff --git a/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java b/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java index 7fd4f659a5..824fba2f2c 100644 --- a/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java +++ b/source/java/org/alfresco/repo/lock/JobLockServiceImpl.java @@ -287,7 +287,7 @@ public class JobLockServiceImpl implements JobLockService // First check the VM if (shutdownListener.isVmShuttingDown()) { - callback.lockReleased(); + callLockReleased(callback); return; } boolean isActive = false; @@ -324,13 +324,13 @@ public class JobLockServiceImpl implements JobLockService try { releaseLock(lockToken, lockQName); + // The callback must be informed as we released the lock automatically + callLockReleased(callback); } catch (LockAcquisitionException e) { // The lock is already gone: job done } - // The callback must be informed - callLockReleased(callback); } else { @@ -338,6 +338,8 @@ public class JobLockServiceImpl implements JobLockService { refreshLock(lockToken, lockQName, timeToLive); // Success. The callback does not need to know. + // NB: Reschedule this task + scheduler.schedule(this, delay, TimeUnit.MILLISECONDS); } catch (LockAcquisitionException e) { diff --git a/source/java/org/alfresco/repo/lock/JobLockServiceTest.java b/source/java/org/alfresco/repo/lock/JobLockServiceTest.java index 015ba4770f..dc4e00aed6 100644 --- a/source/java/org/alfresco/repo/lock/JobLockServiceTest.java +++ b/source/java/org/alfresco/repo/lock/JobLockServiceTest.java @@ -305,20 +305,22 @@ public class JobLockServiceTest extends TestCase final long lockTTL = 1000L; final String lockToken = jobLockService.getLock(lockQName, lockTTL); - final boolean[] released = new boolean[1]; + final int[] checked = new int[1]; + final int[] released = new int[1]; // Immediately-inactive job JobLockRefreshCallback callback = new JobLockRefreshCallback() { @Override public boolean isActive() { + checked[0]++; return false; } @Override public void lockReleased() { - released[0] = true; + released[0]++; } }; @@ -326,7 +328,7 @@ public class JobLockServiceTest extends TestCase // The first refresh will occur in 500ms wait(1000L); // Should have been released by now - assertTrue("Expected lockReleased to have been called", released[0]); + assertTrue("Expected lockReleased to have been called", released[0] > 0); try { jobLockService.getLock(lockQName, lockTTL); @@ -335,6 +337,13 @@ public class JobLockServiceTest extends TestCase { fail("Lock should have been released by callback infrastructure"); } + + // Check that the timed callback is killed properly + int checkedCount = checked[0]; + int releasedCount = released[0]; + wait(2000L); + assertEquals("Lock callback timer was not terminated", checkedCount, checked[0]); + assertEquals("Lock callback timer was not terminated", releasedCount, released[0]); } public synchronized void testLockCallbackReleaseSelf() throws Exception @@ -343,13 +352,15 @@ public class JobLockServiceTest extends TestCase final long lockTTL = 1000L; final String lockToken = jobLockService.getLock(lockQName, lockTTL); - final boolean[] released = new boolean[1]; + final int[] checked = new int[1]; + final int[] released = new int[1]; // Immediately-inactive job, releasing the lock JobLockRefreshCallback callback = new JobLockRefreshCallback() { @Override public boolean isActive() { + checked[0]++; jobLockService.releaseLock(lockToken, lockQName); return false; } @@ -357,15 +368,15 @@ public class JobLockServiceTest extends TestCase @Override public void lockReleased() { - released[0] = true; + released[0]++; } }; jobLockService.refreshLock(lockToken, lockQName, lockTTL, callback); // The first refresh will occur in 500ms wait(1000L); - // Should have been released by now - assertTrue("Expected lockReleased to have been called", released[0]); + // Should NOT get a callback saying that the lock has been released + assertFalse("DID NOT expect lockReleased to have been called", released[0] > 0); try { jobLockService.getLock(lockQName, lockTTL); @@ -374,39 +385,57 @@ public class JobLockServiceTest extends TestCase { fail("Lock should have been released by callback infrastructure"); } + + // Check that the timed callback is killed properly + int checkedCount = checked[0]; + int releasedCount = released[0]; + wait(2000L); + assertEquals("Lock callback timer was not terminated", checkedCount, checked[0]); + assertEquals("Lock callback timer was not terminated", releasedCount, released[0]); } + /** + * Lets job "run" for 3 seconds and checks at 2s and 4s. + */ public synchronized void testLockCallbackReleaseTimed() throws Exception { final QName lockQName = QName.createQName(NAMESPACE, getName()); final long lockTTL = 1000L; + final long lockNow = System.currentTimeMillis(); final String lockToken = jobLockService.getLock(lockQName, lockTTL); - final boolean[] checked = new boolean[1]; - final boolean[] released = new boolean[1]; + final int[] checked = new int[1]; + final int[] released = new int[1]; // Do not release and remain active JobLockRefreshCallback callback = new JobLockRefreshCallback() { @Override public boolean isActive() { - checked[0] = true; - return true; + checked[0]++; + if (System.currentTimeMillis() - lockNow > 3000L) + { + return false; + } + else + { + return true; + } } @Override public void lockReleased() { - released[0] = true; + released[0]++; } }; jobLockService.refreshLock(lockToken, lockQName, lockTTL, callback); // The first refresh will occur in 500ms - wait(1000L); + wait(2000L); - assertTrue("Active check has not occured", checked[0]); - assertFalse("lockReleased should NOT have been called", released[0]); + assertTrue("Expected at least 2 active checks; only got " + checked[0], checked[0] >= 2); + assertFalse("lockReleased should NOT have been called", released[0] > 0); try { jobLockService.getLock(lockQName, lockTTL); @@ -416,5 +445,15 @@ public class JobLockServiceTest extends TestCase { // Expected } + + // Wait for another 2s to be sure that the lock is run to completion + wait(2000L); + + // Check that the timed callback is killed properly + int checkedCount = checked[0]; + int releasedCount = released[0]; + wait(2000L); + assertEquals("Lock callback timer was not terminated", checkedCount, checked[0]); + assertEquals("Lock callback timer was not terminated", releasedCount, released[0]); } }