From 17e87e0f2ce0ac129c085ef36621ea182f4a5159 Mon Sep 17 00:00:00 2001
From: dsyer <dsyer@2e8e966d-6415-0410-94b9-dc4e65435d03>
Date: Thu, 14 Aug 2008 12:34:45 +0000
Subject: [PATCH] BATCH-782: added flag to check that lock is not released
 before it is acquired

---
 archetypes/simple-cli/.project                |  2 +-
 .../core/step/item/ItemOrientedStep.java      | 11 +++++--
 .../item/StepExecutorInterruptionTests.java   | 29 +++++++++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/archetypes/simple-cli/.project b/archetypes/simple-cli/.project
index c663d81a2..fbbe41a0f 100644
--- a/archetypes/simple-cli/.project
+++ b/archetypes/simple-cli/.project
@@ -39,7 +39,7 @@ $ mvn archetype:create -DgroupId=com.mycompany -DartifactId=batch \
 		<link>
 			<name>archetypes-pom.xml</name>
 			<type>1</type>
-			<location>SPRING_BATCH/archetypes/pom.xml</location>
+			<locationURI>SPRING_BATCH/archetypes/pom.xml</locationURI>
 		</link>
 	</linkedResources>
 </projectDescription>
diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java
index d985b8451..57d13c825 100644
--- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java
+++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java
@@ -243,6 +243,8 @@ public class ItemOrientedStep extends AbstractStep {
 
 				TransactionStatus transaction = transactionManager.getTransaction(transactionAttribute);
 
+				boolean locked = false;
+
 				try {
 
 					try {
@@ -265,7 +267,8 @@ public class ItemOrientedStep extends AbstractStep {
 					// to synchronize changes to the step execution (at a
 					// minimum).
 					try {
-						synchronizer.lock(stepExecution);
+							synchronizer.lock(stepExecution);
+							locked = true;
 					}
 					catch (InterruptedException e) {
 						stepExecution.setStatus(BatchStatus.STOPPED);
@@ -345,7 +348,11 @@ public class ItemOrientedStep extends AbstractStep {
 					throw e;
 				}
 				finally {
-					synchronizer.release(stepExecution);
+						// only release the lock if we acquired it
+						if (locked) {
+							synchronizer.release(stepExecution);
+						}
+						locked = false;
 				}
 
 				// Check for interruption after transaction as well, so that
diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StepExecutorInterruptionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StepExecutorInterruptionTests.java
index 495589de3..f306e7999 100644
--- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StepExecutorInterruptionTests.java
+++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StepExecutorInterruptionTests.java
@@ -141,6 +141,35 @@ public class StepExecutorInterruptionTests extends TestCase {
 
 	}
 
+	public void testLockNotReleasedIfChunkFails() throws Exception {
+
+		step.setItemHandler(new SimpleItemHandler(new AbstractItemReader() {
+			public Object read() throws Exception {
+				throw new RuntimeException("Planned!");
+			}
+		}, itemWriter));
+
+		step.setSynchronizer(new StepExecutionSynchronizer() {
+			private boolean locked = false;
+			public void lock(StepExecution stepExecution) throws InterruptedException {
+				locked = true;
+			}
+			public void release(StepExecution stepExecution) {
+				assertTrue("Lock released before it is acquired", locked);
+			}
+		});
+		
+		try {
+			step.execute(stepExecution);
+			fail("Expected planned RuntimeException");
+		} catch (RuntimeException e) {
+			assertEquals("Planned!", e.getMessage());
+		}
+
+		assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
+
+	}
+
 	/**
 	 * @return
 	 */