Open dmatej opened 1 year ago
Assuming non-null wass passed to
https://github.com/eclipse-ee4j/glassfish/blob/f7687e86c1f20682f6218067b1e96d76a1d16e3b/appserver/jdbc/jdbc-ra/jdbc-core/src/main/java/com/sun/gjc/spi/ManagedConnectionImpl.java#L166
the only other assignment in this class I see is
https://github.com/eclipse-ee4j/glassfish/blob/f7687e86c1f20682f6218067b1e96d76a1d16e3b/appserver/jdbc/jdbc-ra/jdbc-core/src/main/java/com/sun/gjc/spi/ManagedConnectionImpl.java#L372
(Unfortunately https://github.com/eclipse-ee4j/glassfish/blob/f7687e86c1f20682f6218067b1e96d76a1d16e3b/appserver/jdbc/jdbc-ra/jdbc-core/src/main/java/com/sun/gjc/spi/ManagedConnectionImpl.java#L91 but I can't locate any other sub-class.)
But given that, could https://github.com/eclipse-ee4j/glassfish/blob/f7687e86c1f20682f6218067b1e96d76a1d16e3b/appserver/jdbc/jdbc-ra/jdbc-core/src/main/java/com/sun/gjc/spi/ManagedConnectionImpl.java#L665 be checkIfValid
ed? This would throw ResourceException
that the clients are prepared for.
Another idea I have is to check the order of operations ... something like if it would do first cleanup and then tried to use nulled field. I don't know yet. Another thing is that in my experience when I fix things like this, something else breaks, because this failed fast, so I expect big fun with it :-D
Just for clarification - the current master causes NPEx in 669 https://github.com/eclipse-ee4j/glassfish/blob/f7687e86c1f20682f6218067b1e96d76a1d16e3b/appserver/jdbc/jdbc-ra/jdbc-core/src/main/java/com/sun/gjc/spi/ManagedConnectionImpl.java#L665-L680
at com.sun.gjc.spi.ManagedConnectionImpl.getActualConnection(ManagedConnectionImpl.java:669)
not as originally reported ManagedConnectionImpl.java:680
.
Right, I probably used to second tab in Konsole. My Eclipse added some @Override
annotations, so it moved.
This issue has been marked as inactive and old and will be closed in 7 days if there is no further activity. If you want the issue to remain open please add a comment
Current master branch, as of 11. January 2023. Tests passed, so it seems this is some consequence of tested behavior, but it would be good to replace the generic NPE by another runtime exception with developer-friendly message.
I lost one hour thinking that I broke it when I was doing changes in WebappCL, now I see the log is same in both branches. You know - "It is Nullpointer, how that could happen, this is not normal!" :-D
Note: Seems to me this will need larger changes too. The connection can be reset, but getter is not prepared for that, and it's clients too. Also I am not sure if it would be possible to replace it with SQLException or some custom runtime exception would be better (no collision with specs?).