TransactionAwareDataSourceProxy leaks connections [SPR-5780]

[spring-projects-issues]

Urs Keller opened SPR-5780 and commented

In TransactionAwareDataSourceProxy isClosed() is not specially handled, under some circumstances the call to this method will create a connection, which is then leaked, since callers are using isClosed() in a finally clause when releasing resources.

We experienced this problem with a connection pool when bounding the number of connections and under high load when the pool becomes exhausted. Under these circumstances the connections are leaking.

If we assume a pool with 1 connection and we are using TransactionAwareDataSourceProxy.
Thread 1) gets the connection and holds it.
Thread 2) tries to get a connection, but since 1) holds it it has a timeout.
We have a context switch to 1) which releases the connection.
We have a context switch to 2). Since it holds a TransactionAwareDataSourceProxy Connection, it will still try to close it. In particular the isCosed method is called and since the target connection is null a new connection is created, which is then leaked.

I tentatively fixed this problem by handling the isClosed when target is null. This passes the tests with which I could reproduce the problem. I'm not sure if there are unwanted side effects, though.

Patch below:

1. Eclipse Workspace Patch 1.0

#P spring-2-5-6-SEC01
Index: src/org/springframework/jdbc/datasource/TransactionAwareDataSourceProxy.java

---

--- src/org/springframework/jdbc/datasource/TransactionAwareDataSourceProxy.java (revision 17121)
+++ src/org/springframework/jdbc/datasource/TransactionAwareDataSourceProxy.java (working copy)
@@ -201,6 +201,12 @@
this.closed = true;
return null;
}
+

// prevent isClosed from opening new connections.

  // since isClosed is usually called in finally blocks

  // this would cause connections to leak.

  else if  (method.getName().equals("isClosed") && this.target==null) {

  return true;

    	}
  
    	if (this.target == null) {
    		if (this.closed) {

---

Affects: 2.5.6

Referenced from: commits d46c1f6

1 votes, 5 watchers

[spring-projects-issues]

Thomas Risberg commented

I have not been able to reproduce this where the last connection is leaked. I have been able to reproduce the part where the connection is null and a new one is created on the isClosed call, but so far they have all been closed eventually.

Can you share your configuration and if possible a simple test case that shows this problem.

-Thomas

[spring-projects-issues]

Urs Keller commented

I had a Jmeter test case in which I was able to reproduce this case, under high load and with a connection pool with a small number of connections.
Basically if the same thread sees its first two attempts (getTransactionIsolation, isClosed) to obtain a fixed connection fail and the third (isClosed) passes, it will leak the connection.

It is possible to reproduce this using a debugger and by throwing an exception manually, where a timeout would have occurred. I found one "constellation" in which connections are leaking:

Setup (I'm working with Eclipse):
In org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy

1. set a conditional breakpoint b1 in the method invoke on line 180: condition: method.getName().equals("isClosed") || method.getName().equals("close")
2. set a breakpoint b2 on line 210 : this.target = DataSourceUtils.doGetConnection(this.targetDataSource); in the if (shouldObtainFixedConnection(..)) block
3. create a disabled watch expression w1: throw new RuntimeException();

Steps:

1. the first time I get to b2 the method is getTransactionIsolation()
2. I simulate a timeout retrieving the connection from the pool using "Reevaluate Watch Expression" on w1
3. I now get to b1 with method name isClosed
4. I continue to b2
5. as in 2) the timout is simulated by evaluating w1
6. I'm getting again to b1 with method name isClosed
7. I get to b2, this time I let the code continue without evaluating w1
8. notice that you never get to b1 with a method close

Short: make the line 210 this.target = DataSourceUtils.doGetConnection(this.targetDataSource); fail the first two times and your connection is never closed.
If only the first call fails, the cleanup works correctly, and you will see a isClosed (opening a connection) followed by a close.

Hope, that helps,
-Urs

[spring-projects-issues]

Juergen Hoeller commented

Since we're tracking the closed state explicitly anyway now, we're simply always returning our handle's closed state from isClosed. This should remove the potential for leaks as well.

Juergen