Review JDBC code for commit/rollback leaks

I didn’t look at the connector yet, but yeah, I suspect so. I’ll look at it before we close it.

https://shibboleth.atlassian.net/browse/JSPT-126 Not currently targeted (since it needs to be 9.0

I’ll look at it before we close it.

AFAICS (I’d appreciate the double check) The injectable thing is the DataSource and that all we ever do is call getConnection() on it.

I’m just about to push the PairwiseIdChange then I’ll assign it pn

I didn’t look at the connector yet, but yeah, I suspect so. I’ll look at it before we close it.

If you want to stick that connection class in shib-support, that’s fine with me. I don’t think it’s an additional dependency, right? JDBC is just part of Java anyway. Or just create a ticket for 6.0 to do it all.

So JDBCPairwiseIdStore is easy although my cut & paste of ConnectionWithLock makes me itchy. We may want to make that a first class object ‘sometime’

I am much less sanguine about the RDBMS connector. In 15+ years we have never been told that it leaks connections but it just grabs a connection from the data source but it neither sets it to auto commit, nor commits it nor rolls it back.

Because it is a data source (not a writeable thing) I’m inclined to let the implementation worry about that and assume that the connection has already been set to auto-commit or is rolled back or something. After all the folks who implemented getConnection() should know the folks (or be the folks) who implement the Connection so if they don’t do the right thing we cannot double guess them and have a case that that is their bug. Right ? (he says being very aware of that OIDC issue)

Heh, same code with the flag on the connection wrapper, so yeah, suspect we want the same assert logic and fixes here.