-1
The use of InternalDriver.activeDriver() to obtain a factory exposes Derby to
potential NullPointerExceptions in a shutdown or error on boot situation. I see
that you saw one in your testing as you have a comment on that in the shutdown
code. Such NPEs will hide the real error from users, making Derby harder to
use.
I think the solution is to move some of the code from
java/engine/org/apache/derby/impl/jdbc/Util.java to be factory methods on
InternalDriver. or methods on EmbedConnection or EmbedConnectionChild. I think
you will find that most, if not all, cases these static Util methods are called
from a JDBC implementation object which has a reference to its InternalDriver
factory. Such a change would then remove the need for special code in the
shutdown case. Basically this move to a factory based allocation for
SQLException obejcts should make the Util class go away.
Also the new patch is missing the test code.
I checked the classes calling using Util to get the SQLException either
already have the reference of InternalDriver or they can be easily
modified to have this reference. There appears to be 1 problem class
EmbedResultSetMetaData.
EmbedResultSetMetaData is being instantiated staticly from many classes
(eg org.apache.derby.diag.StatementDuration, StatementCache). I am not
sure if its safe to assume that InternalDriver variable will be
initialized before these classes are loaded. Same problem will exist if
we use EmbedConnection or EmbedConnectionChild for generating the exception.
Another potential place to convert the old exception to newer one was
handleException in EmbedConnection. Most of the method use this method
to do cleanup before throwing back the exception. But this won't handle
connection related exception (connection doesn't exist, class 08)
anurag
