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

When the Exception is thrown out from the constructor in: AbstractPreparedStatement - line: 151, finalizer tries to remove FBstatement from activeStatements collection and warn appears. [JDBC538] #571

Closed
firebird-issue-importer opened this issue Jul 18, 2018 · 11 comments

Comments

@firebird-issue-importer

Submitted by: Josef Melich (josef)

If you try to executeQuery() on statement e.g.: SELECT MAX(num) FROM nums
And the table "nums" is not exists, the SQLException is thrown in FBConnection - line: 1191 and next command: activeStatements.add(stmt); wont proceed. The problem is, that then the GarbageCollector tries to finalize the uncreated Object and calls method: "void notifyStatementClosed(FBStatement stmt)", which loggs: "log.warn("Specified statement was not created by this connection: " + stmt);" because the uncreated PreparedStatement throws exception before and was not added to the collection: "activeStatements".

Finalization is called when the ancestor constructor of java.lang.Object has ended successfully, refer here: https://stackoverflow.com/questions/14483279/can-finalize-be-called-after-a-constructor-throws-an-exception

Your algorithm simply does not count with the fact that if the exception is throwed in the constructor, the statement will be closed too. (Then the warn appears.)

Thank you for solving :)

Commits: f8c8f29 e467eb5 eadbb71 5c209ff

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 18, 2018

Modified by: @mrotteveel

security: Developers [ 10012 ] =>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 18, 2018

Modified by: @mrotteveel

Fix Version: Jaybird 3.0.5 [ 10873 ]

Fix Version: Jaybird 4 [ 10441 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 18, 2018

Commented by: @mrotteveel

Do you mean when using `Connection.prepareStatement`, because I don't see how this could occur with `executeQuery` on a normal Statement object.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 18, 2018

Commented by: @mrotteveel

The best solution would probably be to move prepare from the constructor, to a separate method, or making registration a side-effect of the constructor. However that has knock-on effects elsewhere, so instead I'll check if it is a prepared statement that is not fully initialized and then not log the warning.

I'll consider a cleaner solution for Jaybird 4 or later.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 18, 2018

Commented by: @mrotteveel

Commit for Jaybird 3: 5c209ff

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 19, 2018

Commented by: Josef Melich (josef)

Yes, sorry for delay, the callstack of calling constructor is here:

at org\.firebirdsql\.jdbc\.FBConnection\.prepareStatement\(FBConnection\.java:1191\)
at org\.firebirdsql\.jdbc\.FBConnection\.prepareStatement\(FBConnection\.java:961\)
at org\.firebirdsql\.jdbc\.FBConnection\.prepareStatement\(FBConnection\.java:915\)
at org\.firebirdsql\.jdbc\.FBConnection\.prepareStatement\(FBConnection\.java:324\)

Can I ask when the version 3.0.5 will come out? Thank you :)

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 21, 2018

Commented by: @mrotteveel

I don't have a definitive release date yet, but probably somewhere in August. If this is really a big issue for you, I could build a snapshot version for you. Let me know if you need one.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 21, 2018

Commented by: @mrotteveel

Committed the same fix for Jaybird 4. Created an improvement ticket to look into a cleaner solution for Jaybird 5.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 21, 2018

Modified by: @mrotteveel

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 23, 2018

Commented by: Josef Melich (josef)

No problem, the August will be fine. :) Thank you.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 7, 2018

Modified by: @mrotteveel

status: Resolved [ 5 ] => Closed [ 6 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants