Deadlock with nested connections and connection pool.

I recently had to debug a deadlock issue with a legacy code that would deadlock when accessing its REST API by multiple threads.
The issue was in the way the REST API was calling Oracle Database and using connections from the pool.
Here is a pseudo code showing the database access used by the REST API:

1 public Result getResult(){
2   Result result = null;
3   try( Connection connection = getConnection(); PreparedStatement ps = connection.prepareStatement(sql), ResultSet rs = ps.executeQuery() ){
4     if ( rs.next() ) {
5       result = new Result();
6       String id = rs.getString(“id”);
7       SubResult subResult = getSubResult(id); //BAD, potential DEADLOCK situation
8       result.setSubResult(subResult);
9       return result;
10     }
11   }catch(Exception e{
12     return null;
13   }
14 }

15 public SubResult getSubResult(String id){
16   SubResult result = null;
17   try( Connection connection = getConnection(); PreparedStatement ps = connection.prepareStatement(sql), ResultSet rs = ps.executeQuery() ){
18     if ( rs.next() ) {
19       result = new SubResult();
20       //update SubResult from ResultSet
21       return result;
22     }
23   }catch(Exception e{
24     return null;
25   }
26 }

27 private Connection getConnection(){
28   return //new connection from the pool
29 }

The REST API call goes through the following steps:

1. Calls geResult().
2. geResult() in turn calls getConnection() on line 3 that will go and try to acquire connection from the pool.
3. Before closing the original connection geResult() will call getSubResult() on line 7 that in turn will try to acquire connection from the pool on line 17.
4. getSubResult() closes its connection.
5. getResult() closes its connection.

All seem to be ok here and the code properly acquires and releases the DB connections from the pool. And this will always work as long as we call geResult() non concurently.
But once we start calling geResult() from multiple threads we will quickly get into a deadlock situation by the following scenario:

1. Multiple threads call geResult() and open all the available connections from the pool by entering line 3.
2. Then the threads will call getSubResult() on line 7 that in turn will try to acquire connection on line 17.
3. Since all connections have been acquired, each thread will sit and wait on line 17 for a new connection to be available that will never happen since they are the ones holding the connections and at the same time not closing them because they are waiting on line 17. And this is how you get a DEADLOCK.

To fix the problem with current code we need to update it to pass the same connection created at line 3 to getSubResult() that will prevent getSubResult() from trying and waiting for a new connection from the pool.

But the best approach is to use frameworks like Spring that will manage connections for us and make sure that only one connection is used per thread (Spring will bind the connection to thread local when using its transaction management facilities via annotations and proxy).

If you are stuck with legacy code like this and cant use Spring then NEVER use nested connections like the above example.

Leave a Reply

You can use these HTML tags

<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>