We use SingleConnectionDataSource with the Oracle driver for integration tests. Although we use the Spring TestContext Framework this sometimes results in commits when we abort tests. We believe this is due to the shutdown hook registered in AbstractApplicationContext#registerShutdownHook() ending up calling SingleConnectionDataSource#destroy() which calls Connection#close(). Calling Connection#close() with an open transaction results in the Oracle driver committing.
This behavior is documented in several places and even mentioned in the official documentation.
https://docs.oracle.com/en/database/oracle/oracle-database/21/jjdbc/JDBC-getting-started.html#GUID-96D6787D-71A6-4282-B17E-0951DED3DEF9
If the auto-commit mode is disabled and you close the connection without explicitly committing or rolling back your last changes, then an implicit COMMIT operation is run.
Calling Connection#close() with an open transaction is not recommended and results in unspecified behavior according to the JDBC specification:
https://docs.oracle.com/en/java/javase/11/docs/api/java.sql/java/sql/Connection.html#close()
It is strongly recommended that an application explicitly commits or rolls back an active transaction prior to calling the close method. If the close method is called and there is an active transaction, the results are implementation-defined.
We would welcome a property, for example named rollbackBeforeClose that behaves something like the following. Maybe with an additional try-catch around Connection#rollback(). An alternative would be to always try to rollback when auto commit is disabled to follow the JDBC reccomendation, however we don't know what the side effects of this would be.
private void closeConnection() {
if (this.target != null) {
try {
if (this.rollbackBeforeClose && !this.target.getAutoCommit()) {
this.target.rollback();
}
this.target.close();
}
catch (Throwable ex) {
logger.info("Could not close shared JDBC Connection", ex);
}
}
}
We would be willing to work on a PR.
Comment From: snicoll
rollbackBeforeClose is test-specific and not a good candidate for SingleConnectionDataSource.
Calling Connection#close() with an open transaction is not recommended and results in unspecified behavior according to the JDBC specification:
Yes, and you haven't described who's managing the boundary of that transaction (or if there is a transaction at all). The TCF will rollback at the end of a test by default so I can only assume that you're not using that?
Comment From: marschall
The TCF will rollback at the end of a test by default so I can only assume that you're not using that?
We are using that but as you say the TCF only rolls back upon test completion. If the JVM terminates before this then the shutdown hook (registered in AbstractApplicationContext#registerShutdownHook()) is executed which closes the application context which ends up calling SingleConnectionDataSource#destroy() which calls Connection#close().
This can happen when for example a developer hits Terminate in the Eclipse debugger which then results in a commit of all open transactions.
Comment From: snicoll
Thanks for the feedback. I wonder if that would make sense to register a similar mechanism when the test runs to make sure that the rollback happens if the test is stopped mid-flight. To me, that would be a more consolidated answer than your suggestion of rollbackBeforeClose. I am also puzzled that we didn't have more reports of this behavior.
Flagging for team attention to see what the rest of the team thinks.
Comment From: marschall
Yes, there are other ways how this could be solved. TCF could roll back all in flight transactions or the transaction manager could roll back all in flight transactions.
Comment From: jhoeller
Since SingleConnectionDataSource is primarily for testing environments anyway and the Oracle driver behavior can be hard to deal with indeed, I'm adding a rollbackBeforeClose property.
Being at it, I've also made SingleConnectionDataSource implement AutoCloseable for use with a try-with-resource statement.