Connections not closed on Shutdown

[johannesherr]

There seems to be a race condition when a pool is created and immediately shutdown, that leaves connections open.

Tested with HikariCP 1.3.8 and "PostgreSQL 9.3.4, compiled by Visual C++ build 1600, 64-bit". DataSource is org.postgresql.ds.PGSimpleDataSource.

To reproduce create a HikariDataSource and shut it down immediately. Querying the number of open connections afterward SELECT count(*) FROM pg_stat_activity; shows 21 (20 is the my configured Hikari pool size). Its sufficient to sleep for a second before shuting down to make the problem go away.

HikariDataSource dataSource = new HikariDataSource(config);
// Thread.sleep(1_000);  // will make the problem vanish 
dataSource.shutdown();
// 20 connections left open

The problem might be in line 354 in com.zaxxer.hikari.pool.HikariPool. The method closeIdleConnections as its name indicates closes only idle connections. If one sets a conditional break point on line 355 with the condition list.size() != connectionBag.size() one sees the the list of idle connections is empty and the list of connections has size 20. I am not sure what the strategy is for closing 'non-idle' connections on pool shutdown.

To give some context I run into this problem when a test suite started failing yesterday (too many open connections db error) . The tests of course do not close pools immediately after creating them, that is just for reproduceing the problem. They might however run just a few statements before closing a pool. I am also wondering if a connection might be left open, when it used before the pool is shut down. It might be non-idle at that point?

Thanks a lot for your impressing work. Hope this helps.

[brettwooldridge]

Thanks for finding this. I'll add a unit test and work on a fix.

[brettwooldridge]

If you don't mind, it would be great if you can test the 1.3.9-SNAPSHOT build to see if your issue is fixed (after removing your sleep call).

git clone https://github.com/brettwooldridge/HikariCP.git
cd HikariCP
mvn install

Then use 1.3.9-SNAPSHOT as a maven dependency or use the jar from the target directory.

[johannesherr]

Thank you for the quick response.

I have tested the new version. 2 remarks:

1. I do not see a increase in connections any more. So this problem seems to be fixed.
2. However there seems to be a thread leak. Each of my tests leaves a "HikariCP connection filler" thread behind. Since these are running with 100% cpu, tests start failing shortly after.

I have looked into the thread leak problem. I think the cause is, that the "filler"/AddConnection-Thread (create in HikariPool.addBagItem()), does not realize that its thread pool has been shut down. You close the thread pool via shutdownNow which interrupts its threads. In some cases a currently running filler-thread can get stuck between checking its while condition (which tells it, it needs to add more connections) and calling addConnection (which immediately returns because is checks isShutdown in a guard clause). There is no method call that would throw an interrupted exception on this code path. I have added a debug print statement Thread.currentThread().isInterrupted() which prints thousand of trues when running my testsuite. Adding a Thread.currentThread().isInterrupted() to the thread loop condition however has not worked reliably. I haven't looked at that in detail, but I assume, that catch (Exception e) clause in addConnection might swallow the InterruptedException and thereby the interrupted flag, just guessing here.

A working fix has been to add a !isShutdown check also in the thread loop. I have added a test case. However this counting of threads is quite error prone. So maybe the test is not worth keeping. It demonstrates the problem however.

I have tried to create a pull request with an added test case for the thread leak and a possible fix. I am new to this github thing, so I hope I haven't messed up. :)