Bug #64954 Deadlock on Connection close if another Thread does execute a PreparedStatement
Submitted: 12 Apr 2012 11:59 Modified: 24 Jul 2012 15:52
Reporter: Torsten Krah Email Updates:
Status: Won't fix Impact on me:
None 
Category:Connector / J Severity:S1 (Critical)
Version:5.1.19 OS:Linux
Assigned to: Alexander Soklakov CPU Architecture:Any

[12 Apr 2012 11:59] Torsten Krah
Description:
Imagine this scenario:

In Thread A a Pooled Connection is used to create a prepared statement which should be executed.

In Thread B where the pool is managed, the decision is made, that the connection was leased for too long (e.g. Thread A got stuck and i want to recover my connection object) or my app is being shutdown and i want to forcefully close all connections.

So Thread B gets the connection and calls close on the object.
At this point a deadlock does occur like in the stacktrace shown:

Found one Java-level deadlock:
=============================
"BuildItemIndex":
  waiting to lock monitor 0x0000000002cd3250 (object 0x00000000fe4a9bd0, a com.mysql.jdbc.PreparedStatement),
  which is held by "pool-2-thread-13"
"pool-2-thread-13":
  waiting to lock monitor 0x0000000002cd32f8 (object 0x00000000e0107748, a com.mysql.jdbc.JDBC4Connection),
  which is held by "BuildItemIndex"

Java stack information for the threads listed above:
===================================================
"BuildItemIndex":
	at com.mysql.jdbc.PreparedStatement.realClose(PreparedStatement.java:3085)
	- waiting to lock <0x00000000fe4a9bd0> (a com.mysql.jdbc.PreparedStatement)
	at com.mysql.jdbc.ConnectionImpl.closeAllOpenStatements(ConnectionImpl.java:1628)
	at com.mysql.jdbc.ConnectionImpl.realClose(ConnectionImpl.java:4388)
	at com.mysql.jdbc.ConnectionImpl.close(ConnectionImpl.java:1601)
	- locked <0x00000000e0107748> (a com.mysql.jdbc.JDBC4Connection)
	at de.test.util.db.ConnectionPool.closeConnections(ConnectionPool.java:353)
	at de.test.util.db.PersistStore.executeQuery(PersistStore.java:866)
        ...
"pool-2-thread-13":
	at com.mysql.jdbc.PreparedStatement.executeQuery(PreparedStatement.java:2210)
	- waiting to lock <0x00000000e0107748> (a com.mysql.jdbc.JDBC4Connection)
	- locked <0x00000000fe4a9bd0> (a com.mysql.jdbc.PreparedStatement)
	....

Looking at the code it seems obvious:

close is synchronized on the connection object itself.
The prepared statement does synchronize too at this object when calling executeQuery:

###############
The prepared statement does synchronize too at this object:

		// We need to execute this all together
		// So synchronize on the Connection's mutex (because
		// even queries going through there synchronize
		// on the same mutex.
		synchronized (locallyScopedConn) {
###############

But close does call closeAllOpenStatements which calls realClose on the Statement, but the mutex (object itself) is already hold - deadlock.

So how to cope with this use case?
Its not so uncommon that thread pools wants to forcefully close borrowed connections (shutdown in certain time) or borrow time too long, to get back the connection.

How to repeat:
Create 2 threads which share one Connection.
In one Thread create a prepared statement and execute it - set a breakpoint before the implicit connection mutex (See above) is aquired.
In the other Thread call close on the shared Connection - simulating forced close.
Resume the break point from the prepared statement.

Watch the deadlock.

Suggested fix:
PreparedStatement should use LockObjects instead of implicit mutexes by synchronize.
You will be able to use tryLock() or use lock with a timeout and can make the current Thread sleep to get rid of currently hold monitors to let the deadlock go "away" and be able to call close later on.
If its not possible to close the connection in a certain time, you may throw an special exception to indicate this to the callee - maybe he is able to stall the operation to a later point.
[12 Apr 2012 12:00] Torsten Krah
StackTrace should read:

Found one Java-level deadlock:
=============================
"BuildItemIndex":
  waiting to lock monitor 0x0000000002cd3250 (object 0x00000000fe4a9bd0, a com.mysql.jdbc.PreparedStatement),
  which is held by "pool-2-thread-13"
"pool-2-thread-13":
  waiting to lock monitor 0x0000000002cd32f8 (object 0x00000000e0107748, a com.mysql.jdbc.JDBC4Connection),
  which is held by "BuildItemIndex"

Java stack information for the threads listed above:
===================================================
"FRC-BuildItemIndex":
	at com.mysql.jdbc.PreparedStatement.realClose(PreparedStatement.java:3085)
	- waiting to lock <0x00000000fe4a9bd0> (a com.mysql.jdbc.PreparedStatement)
	at com.mysql.jdbc.ConnectionImpl.closeAllOpenStatements(ConnectionImpl.java:1628)
	at com.mysql.jdbc.ConnectionImpl.realClose(ConnectionImpl.java:4388)
	at com.mysql.jdbc.ConnectionImpl.close(ConnectionImpl.java:1601)
	- locked <0x00000000e0107748> (a com.mysql.jdbc.JDBC4Connection)
	at de.test.util.db.ConnectionPool.closeConnections(ConnectionPool.java:353)
        ...
"pool-2-thread-13":
	at com.mysql.jdbc.PreparedStatement.executeQuery(PreparedStatement.java:2210)
	- waiting to lock <0x00000000e0107748> (a com.mysql.jdbc.JDBC4Connection)
	- locked <0x00000000fe4a9bd0> (a com.mysql.jdbc.PreparedStatement)
	at de.test.util.db.PersistStore.executeQuery(PersistStore.java:866)
	....
[12 Apr 2012 14:24] Mark Matthews
This is precisely the reason the JDBC-4.0 spec added the abort() method. There is no foolproof way to implement the semantics of Connection.close() *and* never have deadlocks.

The abort() method is designed to be used in these cases (it takes no locks, but doesn't attempt to clean up currently open statements, etc).

Have you asked your connection pool vendor why they're not using the abort() method?
[12 Apr 2012 15:31] Torsten Krah
abort(Executor) is added with JDBC-4.1 feature set.
I do not see any abort at JDBC-4.0.

I am the vendor of my pool itself, but even the tomcat pool people does "still" write code against JDBC-4.0 (JDK 6, as i do) - there is no abort method there.

Looking at JDBC4Connection or ConnectionImpl of mysql connector/j i do not see any abort(Executor) impl either - does the connector support abort already?

As of the time JDK 7+ will be unlikely to be the only supported JDK - how to solve this use case with JDBC-4.0, the driver itself should be able to handle this, shouldn't it?
[24 Jul 2012 15:52] Mark Matthews
The abort() method was added in JDK-7 for this very situation because it was not addressable within the spec as it stood. You can call abort() on 5.1.21 (it is implemented), even without JDK-7, if you can get to the vendor connection (assuming you can, since it's your pool implementation).

We don't plan on complicating the locking logic to address this problem given that it's addressed in JDK7, and those methods are available to callers in older versions of the JDK, especially given there are classes outside the scope of the driver (Socket and friends) that could also block/lock that we don't control.