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

commit(true) leaks Statements/cursors when using connectionPool #202

Open mbknor opened 9 years ago

mbknor commented 9 years ago

sql2o 1.5.2 and 1.5.4

If using beginTransaction and createQuery without explicit closing, commit with closeConnection=true will leak statements when using pooled connections.

I'm using Oracle and get "ORA-01000: maximum open cursors exceeded"

The problem is that Connection.commit(true) uses closeJdbcConnection() instead of close() which would have closed Statements before closing connection.

Example:

  def insertPersistentReprList(dtoList: Seq[JournalEntryDto]) {

    val sql = "insert into ...."

    // Insert the whole list in one transaction
    val c = sql2o.beginTransaction()
    try {

      dtoList.foreach {
        dto =>
          val insert = c.createQuery(sql)
          try {

            insert.addParameter("field", "value").executeUpdate

          } finally {
            // Without this statement-close, we would leak statements resulting in ORA-01000: maximum open cursors exceeded
            insert.close()
          }
      }

      // All inserts executed - commit transaction
      c.commit(true)
    } catch {
      case e:Throwable =>
        c.rollback(true)
        throw e
    }

  }
mbknor commented 9 years ago

Here is the workaround I used: https://github.com/NextGenTel/akka-tools/commit/95baa15a1c7e92c9160557943784c66465443a0a

Marco-Sulla commented 5 years ago

You have to use the try-with-resources.

rlfaber commented 2 years ago

I just reproduced this issue on my code, testing sql2o for the first time. I found that the Quarkus Agroal connection pooling is showing leak detection warnings (and I trust it from my own experiences writing a Jdbc layer).
Agroal shows this warning: Datasource '': JDBC resources leaked: 2 ResultSet(s) and 2 Statement(s)

The issue only happens with transactions (sql2o.beginTransaction()), and it is fixed with the commit above (5225455).
But it is not merged into the master branch... I'm wondering why? This is a very important fix in my mind.
Just wondering when this will make it to the master branch... and then I am eager for a release with this. Based on my testing... this is a very good and critical fix and is solved with the above commit.