revised JMS CachedConnectionFactory to avoid unnecessary rollback calls on Session return (SPR-8437); fixed JMS CachedConnectionFactory to fully synchronize its Session list (SPR-8436)

jhoeller · jhoeller · commit 168d9648b957 · 2011-06-13T21:42:11.000Z
diff --git a/org.springframework.jms/src/main/java/org/springframework/jms/connection/CachingConnectionFactory.java b/org.springframework.jms/src/main/java/org/springframework/jms/connection/CachingConnectionFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 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.
@@ -312,7 +312,7 @@ else if (methodName.equals("getTargetSession")) {
 			else if (methodName.equals("commit") || methodName.equals("rollback")) {
 				this.transactionOpen = false;
 			}
-			else {
+			else if (methodName.startsWith("create")) {
 				this.transactionOpen = true;
 				if (isCacheProducers() && (methodName.equals("createProducer") ||
 						methodName.equals("createSender") || methodName.equals("createPublisher"))) {
@@ -408,11 +408,15 @@ private void logicalClose(Session proxy) throws JMSException {
 				}
 			}
 			// Allow for multiple close calls...
-			if (!this.sessionList.contains(proxy)) {
-				if (logger.isTraceEnabled()) {
-					logger.trace("Returning cached Session: " + this.target);
+			boolean returned = false;
+			synchronized (this.sessionList) {
+				if (!this.sessionList.contains(proxy)) {
+					this.sessionList.addLast(proxy);
+					returned = true;
 				}
-				this.sessionList.addLast(proxy);
+			}
+			if (returned && logger.isTraceEnabled()) {
+				logger.trace("Returned cached Session: " + this.target);
 			}
 		}
 

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2010 the original author or authors.`
	`2`	`+ * Copyright 2002-2011 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.`
`@@ -312,7 +312,7 @@ else if (methodName.equals("getTargetSession")) {`
`312`	`312`	`else if (methodName.equals("commit") \|\| methodName.equals("rollback")) {`
`313`	`313`	`this.transactionOpen = false;`
`314`	`314`	`}`
`315`		`- else {`
	`315`	`+ else if (methodName.startsWith("create")) {`
`316`	`316`	`this.transactionOpen = true;`
`317`	`317`	`if (isCacheProducers() && (methodName.equals("createProducer") \|\|`
`318`	`318`	`methodName.equals("createSender") \|\| methodName.equals("createPublisher"))) {`
`@@ -408,11 +408,15 @@ private void logicalClose(Session proxy) throws JMSException {`
`408`	`408`	`}`
`409`	`409`	`}`
`410`	`410`	`// Allow for multiple close calls...`
`411`		`- if (!this.sessionList.contains(proxy)) {`
`412`		`- if (logger.isTraceEnabled()) {`
`413`		`- logger.trace("Returning cached Session: " + this.target);`
	`411`	`+ boolean returned = false;`
	`412`	`+ synchronized (this.sessionList) {`
	`413`	`+ if (!this.sessionList.contains(proxy)) {`
	`414`	`+ this.sessionList.addLast(proxy);`
	`415`	`+ returned = true;`
`414`	`416`	`}`
`415`		`- this.sessionList.addLast(proxy);`
	`417`	`+ }`
	`418`	`+ if (returned && logger.isTraceEnabled()) {`
	`419`	`+ logger.trace("Returned cached Session: " + this.target);`
`416`	`420`	`}`
`417`	`421`	`}`
`418`	`422`