Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AF-1149: Dashbuilder not closing ResultSets and Statements #420

Merged
merged 1 commit into from Apr 23, 2018

Conversation

dgutierr
Copy link
Member

@jhrcek Fix + tests.

As this fix forces to close the opened result sets, the app server no longer warns or auto closes them, regardless the "track-statements" setting of the data source.

Copy link

@martinweiler martinweiler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgutierr - thanks for this fix, I tested it with the following results:

  • track-statements=false - no more ORA-01000 errors 👍
  • track-statements=true - still seeing WARN msgs from JCA about open statements in JDBCUtils.executeQuery ( line 75) 👎

Is this something that you could take another look at, please?

Closing a result set you left open! Please close it yourself.: java.lang.Throwable: STACKTRACE
at org.jboss.jca.adapters.jdbc.WrappedStatement.registerResultSet(WrappedStatement.java:1357)
at org.jboss.jca.adapters.jdbc.WrappedStatement.executeQuery(WrappedStatement.java:345)
at org.dashbuilder.dataprovider.sql.JDBCUtils.executeQuery(JDBCUtils.java:75) [dashbuilder-dataset-sql-0.5.0.Final-redhat-15-RHBPMS-5121.jar:0.5.0.Final-redhat-15-RHBPMS-5121]

@dgutierr
Copy link
Member Author

dgutierr commented Apr 19, 2018

@martinweiler Now should be fixed. I verified it on EAP with track-statements=true. The solution was a bit tricky tough. As the ironjacamar's WrappedConnection and WrappedStatement classes are implemented, it forces the client code to hold all the references to any ResultSet and Statement instances created so that they can be properly closed.

Please give it another try.

@martinweiler
Copy link

@dgutierr perfect, I just tried your new fix and it works as expected. Thank you!

@dgutierr
Copy link
Member Author

dgutierr commented Apr 19, 2018

@martinweiler Great thanx.

@kkufova Ready for QA :-)

@kkufova
Copy link
Contributor

kkufova commented Apr 23, 2018

jenkins please retest this

Copy link
Contributor

@kkufova kkufova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dgutierr, thanks!

@dgutierr dgutierr merged commit c9df8b4 into dashbuilder:0.5.x Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants