Consistent reset of resource holders on doBegin failure

jhoeller · jhoeller · commit 9758bc783343 · 2014-10-07T00:06:49.000+02:00
Issue: SPR-12280 (cherry picked from commit e58b33a)
diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceTransactionManager.java
@@ -179,7 +179,7 @@ protected Object doGetTransaction() {
 		DataSourceTransactionObject txObject = new DataSourceTransactionObject();
 		txObject.setSavepointAllowed(isNestedTransactionAllowed());
 		ConnectionHolder conHolder =
-			(ConnectionHolder) TransactionSynchronizationManager.getResource(this.dataSource);
+				(ConnectionHolder) TransactionSynchronizationManager.getResource(this.dataSource);
 		txObject.setConnectionHolder(conHolder, false);
 		return txObject;
 	}
@@ -238,7 +238,10 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 		}
 
 		catch (Throwable ex) {
-			DataSourceUtils.releaseConnection(con, this.dataSource);
+			if (txObject.isNewConnectionHolder()) {
+				DataSourceUtils.releaseConnection(con, this.dataSource);
+				txObject.setConnectionHolder(null, false);
+			}
 			throw new CannotCreateTransactionException("Could not open JDBC Connection for transaction", ex);
 		}
 	}
diff --git a/spring-jms/src/main/java/org/springframework/jms/connection/JmsTransactionManager.java b/spring-jms/src/main/java/org/springframework/jms/connection/JmsTransactionManager.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -178,6 +178,7 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 		if (definition.getIsolationLevel() != TransactionDefinition.ISOLATION_DEFAULT) {
 			throw new InvalidIsolationLevelException("JMS does not support an isolation level concept");
 		}
+
 		JmsTransactionObject txObject = (JmsTransactionObject) transaction;
 		Connection con = null;
 		Session session = null;
@@ -193,10 +194,17 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 			if (timeout != TransactionDefinition.TIMEOUT_DEFAULT) {
 				txObject.getResourceHolder().setTimeoutInSeconds(timeout);
 			}
-			TransactionSynchronizationManager.bindResource(
-					getConnectionFactory(), txObject.getResourceHolder());
+			TransactionSynchronizationManager.bindResource(getConnectionFactory(), txObject.getResourceHolder());
 		}
 		catch (Throwable ex) {
+			if (session != null) {
+				try {
+					session.close();
+				}
+				catch (Throwable ex2) {
+					// ignore
+				}
+			}
 			if (con != null) {
 				try {
 					con.close();
diff --git a/spring-orm-hibernate4/src/main/java/org/springframework/orm/hibernate4/HibernateTransactionManager.java b/spring-orm-hibernate4/src/main/java/org/springframework/orm/hibernate4/HibernateTransactionManager.java
@@ -513,6 +513,7 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 				}
 				finally {
 					SessionFactoryUtils.closeSession(session);
+					txObject.setSessionHolder(null);
 				}
 			}
 			throw new CannotCreateTransactionException("Could not open Hibernate Session for transaction", ex);
diff --git a/spring-orm/src/main/java/org/springframework/orm/hibernate3/HibernateTransactionManager.java b/spring-orm/src/main/java/org/springframework/orm/hibernate3/HibernateTransactionManager.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -596,6 +596,7 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 				}
 				finally {
 					SessionFactoryUtils.closeSession(session);
+					txObject.setSessionHolder(null);
 				}
 			}
 			throw new CannotCreateTransactionException("Could not open Hibernate Session for transaction", ex);
diff --git a/spring-orm/src/main/java/org/springframework/orm/jdo/JdoTransactionManager.java b/spring-orm/src/main/java/org/springframework/orm/jdo/JdoTransactionManager.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -341,15 +341,16 @@ public int getTimeout() {
 						conHolder.setTimeoutInSeconds(timeoutToUse);
 					}
 					if (logger.isDebugEnabled()) {
-						logger.debug("Exposing JDO transaction as JDBC transaction [" + conHolder.getConnectionHandle() + "]");
+						logger.debug("Exposing JDO transaction as JDBC transaction [" +
+								conHolder.getConnectionHandle() + "]");
 					}
 					TransactionSynchronizationManager.bindResource(getDataSource(), conHolder);
 					txObject.setConnectionHolder(conHolder);
 				}
 				else {
 					if (logger.isDebugEnabled()) {
-						logger.debug("Not exposing JDO transaction [" + pm + "] as JDBC transaction because JdoDialect [" +
-								getJdoDialect() + "] does not support JDBC Connection retrieval");
+						logger.debug("Not exposing JDO transaction [" + pm + "] as JDBC transaction because " +
+								"JdoDialect [" + getJdoDialect() + "] does not support JDBC Connection retrieval");
 					}
 				}
 			}
@@ -391,6 +392,7 @@ protected void closePersistenceManagerAfterFailedBegin(JdoTransactionObject txOb
 			finally {
 				PersistenceManagerFactoryUtils.releasePersistenceManager(pm, getPersistenceManagerFactory());
 			}
+			txObject.setPersistenceManagerHolder(null, false);
 		}
 	}
 
diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java b/spring-orm/src/main/java/org/springframework/orm/jpa/JpaTransactionManager.java
@@ -331,8 +331,8 @@ protected Object doGetTransaction() {
 				TransactionSynchronizationManager.getResource(getEntityManagerFactory());
 		if (emHolder != null) {
 			if (logger.isDebugEnabled()) {
-				logger.debug("Found thread-bound EntityManager [" +
-						emHolder.getEntityManager() + "] for JPA transaction");
+				logger.debug("Found thread-bound EntityManager [" + emHolder.getEntityManager() +
+						"] for JPA transaction");
 			}
 			txObject.setEntityManagerHolder(emHolder, false);
 		}
@@ -400,15 +400,16 @@ public int getTimeout() {
 						conHolder.setTimeoutInSeconds(timeoutToUse);
 					}
 					if (logger.isDebugEnabled()) {
-						logger.debug("Exposing JPA transaction as JDBC transaction [" + conHolder.getConnectionHandle() + "]");
+						logger.debug("Exposing JPA transaction as JDBC transaction [" +
+								conHolder.getConnectionHandle() + "]");
 					}
 					TransactionSynchronizationManager.bindResource(getDataSource(), conHolder);
 					txObject.setConnectionHolder(conHolder);
 				}
 				else {
 					if (logger.isDebugEnabled()) {
-						logger.debug("Not exposing JPA transaction [" + em + "] as JDBC transaction because JpaDialect [" +
-								getJpaDialect() + "] does not support JDBC Connection retrieval");
+						logger.debug("Not exposing JPA transaction [" + em + "] as JDBC transaction because " +
+								"JpaDialect [" + getJpaDialect() + "] does not support JDBC Connection retrieval");
 					}
 				}
 			}
@@ -467,6 +468,7 @@ protected void closeEntityManagerAfterFailedBegin(JpaTransactionObject txObject)
 			finally {
 				EntityManagerFactoryUtils.closeEntityManager(em);
 			}
+			txObject.setEntityManagerHolder(null, false);
 		}
 	}
 
diff --git a/spring-tx/src/main/java/org/springframework/jca/cci/connection/CciLocalTransactionManager.java b/spring-tx/src/main/java/org/springframework/jca/cci/connection/CciLocalTransactionManager.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -126,7 +126,7 @@ public Object getResourceFactory() {
 	protected Object doGetTransaction() {
 		CciLocalTransactionObject txObject = new CciLocalTransactionObject();
 		ConnectionHolder conHolder =
-			(ConnectionHolder) TransactionSynchronizationManager.getResource(getConnectionFactory());
+				(ConnectionHolder) TransactionSynchronizationManager.getResource(getConnectionFactory());
 		txObject.setConnectionHolder(conHolder);
 		return txObject;
 	}
@@ -159,7 +159,6 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {
 			}
 			TransactionSynchronizationManager.bindResource(getConnectionFactory(), txObject.getConnectionHolder());
 		}
-
 		catch (NotSupportedException ex) {
 			ConnectionFactoryUtils.releaseConnection(con, getConnectionFactory());
 			throw new CannotCreateTransactionException("CCI Connection does not support local transactions", ex);
@@ -268,7 +267,7 @@ public void setConnectionHolder(ConnectionHolder connectionHolder) {
 		}
 
 		public ConnectionHolder getConnectionHolder() {
-			return connectionHolder;
+			return this.connectionHolder;
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,7 @@ protected Object doGetTransaction() {`
`179`	`179`	`DataSourceTransactionObject txObject = new DataSourceTransactionObject();`
`180`	`180`	`txObject.setSavepointAllowed(isNestedTransactionAllowed());`
`181`	`181`	`ConnectionHolder conHolder =`
`182`		`- (ConnectionHolder) TransactionSynchronizationManager.getResource(this.dataSource);`
	`182`	`+ (ConnectionHolder) TransactionSynchronizationManager.getResource(this.dataSource);`
`183`	`183`	`txObject.setConnectionHolder(conHolder, false);`
`184`	`184`	`return txObject;`
`185`	`185`	`}`
`@@ -238,7 +238,10 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {`
`238`	`238`	`}`
`239`	`239`
`240`	`240`	`catch (Throwable ex) {`
`241`		`- DataSourceUtils.releaseConnection(con, this.dataSource);`
	`241`	`+ if (txObject.isNewConnectionHolder()) {`
	`242`	`+ DataSourceUtils.releaseConnection(con, this.dataSource);`
	`243`	`+ txObject.setConnectionHolder(null, false);`
	`244`	`+ }`
`242`	`245`	`throw new CannotCreateTransactionException("Could not open JDBC Connection for transaction", ex);`
`243`	`246`	`}`
`244`	`247`	`}`
Original file line number	Diff line number	Diff line change
`@@ -513,6 +513,7 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {`
`513`	`513`	`}`
`514`	`514`	`finally {`
`515`	`515`	`SessionFactoryUtils.closeSession(session);`
	`516`	`+ txObject.setSessionHolder(null);`
`516`	`517`	`}`
`517`	`518`	`}`
`518`	`519`	`throw new CannotCreateTransactionException("Could not open Hibernate Session for transaction", ex);`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2013 the original author or authors.`
	`2`	`+ * Copyright 2002-2014 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -596,6 +596,7 @@ protected void doBegin(Object transaction, TransactionDefinition definition) {`
`596`	`596`	`}`
`597`	`597`	`finally {`
`598`	`598`	`SessionFactoryUtils.closeSession(session);`
	`599`	`+ txObject.setSessionHolder(null);`
`599`	`600`	`}`
`600`	`601`	`}`
`601`	`602`	`throw new CannotCreateTransactionException("Could not open Hibernate Session for transaction", ex);`
Original file line number	Diff line number	Diff line change
`@@ -331,8 +331,8 @@ protected Object doGetTransaction() {`
`331`	`331`	`TransactionSynchronizationManager.getResource(getEntityManagerFactory());`
`332`	`332`	`if (emHolder != null) {`
`333`	`333`	`if (logger.isDebugEnabled()) {`
`334`		`- logger.debug("Found thread-bound EntityManager [" +`
`335`		`- emHolder.getEntityManager() + "] for JPA transaction");`
	`334`	`+ logger.debug("Found thread-bound EntityManager [" + emHolder.getEntityManager() +`
	`335`	`+ "] for JPA transaction");`
`336`	`336`	`}`
`337`	`337`	`txObject.setEntityManagerHolder(emHolder, false);`
`338`	`338`	`}`
`@@ -400,15 +400,16 @@ public int getTimeout() {`
`400`	`400`	`conHolder.setTimeoutInSeconds(timeoutToUse);`
`401`	`401`	`}`
`402`	`402`	`if (logger.isDebugEnabled()) {`
`403`		`- logger.debug("Exposing JPA transaction as JDBC transaction [" + conHolder.getConnectionHandle() + "]");`
	`403`	`+ logger.debug("Exposing JPA transaction as JDBC transaction [" +`
	`404`	`+ conHolder.getConnectionHandle() + "]");`
`404`	`405`	`}`
`405`	`406`	`TransactionSynchronizationManager.bindResource(getDataSource(), conHolder);`
`406`	`407`	`txObject.setConnectionHolder(conHolder);`
`407`	`408`	`}`
`408`	`409`	`else {`
`409`	`410`	`if (logger.isDebugEnabled()) {`
`410`		`- logger.debug("Not exposing JPA transaction [" + em + "] as JDBC transaction because JpaDialect [" +`
`411`		`- getJpaDialect() + "] does not support JDBC Connection retrieval");`
	`411`	`+ logger.debug("Not exposing JPA transaction [" + em + "] as JDBC transaction because " +`
	`412`	`+ "JpaDialect [" + getJpaDialect() + "] does not support JDBC Connection retrieval");`
`412`	`413`	`}`
`413`	`414`	`}`
`414`	`415`	`}`
`@@ -467,6 +468,7 @@ protected void closeEntityManagerAfterFailedBegin(JpaTransactionObject txObject)`
`467`	`468`	`finally {`
`468`	`469`	`EntityManagerFactoryUtils.closeEntityManager(em);`
`469`	`470`	`}`
	`471`	`+ txObject.setEntityManagerHolder(null, false);`
`470`	`472`	`}`
`471`	`473`	`}`
`472`	`474`