search

aaberg / sql2o

sql2o is a small library, which makes it easy to convert the result of your sql-statements into objects. No resultset hacking required. Kind of like an orm, but without the sql-generation capabilities. Supports named parameters.
http://sql2o.org
MIT License
1.15k stars 229 forks source link

connection: fix statement leak from commiting transactions #300

Open Adam- opened 6 years ago

Adam- commented 6 years ago

Closing the connection with closeJdbcConnection() prevented close() from closing registered statements. Closes #202.

rlfaber commented 2 years ago

Sorry to repeat my comment from #202, but I have found this fix to be a good fix for a problem I had with a very different test case than others perhaps. I was testing sql2o with transactions using 1.6.0 on Quarkus with Agroal connection polling and noticed Agroal leak warnings like: Datasource '': JDBC resources leaked: 2 ResultSet(s) and 2 Statement(s). With this fix (#300), however, the warnings went away. I am eager to see this get into master... and also eager for the next release with it... as this seems to me like a very important fix.

Adam- commented 2 years ago

Sorry to repeat my comment from #202, but I have found this fix to be a good fix for a problem I had with a very different test case than others perhaps. I was testing sql2o with transactions using 1.6.0 on Quarkus with Agroal connection polling and noticed Agroal leak warnings like: Datasource '': JDBC resources leaked: 2 ResultSet(s) and 2 Statement(s). With this fix (#300), however, the warnings went away. I am eager to see this get into master... and also eager for the next release with it... as this seems to me like a very important fix.

I was able to work around this in my code via:

query.executeBatch();
con.commit(false);

at every place that I open a transaction

rlfaber commented 2 years ago

  1. There are still leaks with the above suggestion (batch), and I suspect the important difference is that I am testing with a quality leak detection connection pool, Quarkus Agroal, which is much better at leak detection than some other connection poolers (e.g. dbcp). I have found Agroal excellent at finding leaks in my own raw JDBC. https://agroal.github.io/. What connection pooler were you testing with?

  2. A second issue is that using batch is not a sufficient solution (even if it did work).

  3. I applied this pull request to sql2o and it solves the transaction leak issue!