RFR 8060068 : Remove the static initializer block from DriverManager
Mandy Chung
mandy.chung at oracle.com
Tue Dec 2 21:12:38 UTC 2014
On 12/2/14 12:30 PM, Lance Andersen wrote:
>
>> So it may be better to have a getter method to ensure driver classes are
>> loaded as well as return the registeredDrivers copy-on-write-arraylist.
>> i.e. you could rename initDriversIfNeeded to getRegisteredDrivers and
>> replace
>> for(DriverInfo aDriver : registeredDrivers) {
>>
>> with
>> for(DriverInfo aDriver : getRegisteredDrivers())
> I can do that if you think that is a cleaner approach? I am flexible :-)
I think so. It wasn't obvious to me at the beginning when you need to
ensure drivers are initialized and whether any place needs to call
initDirversIfNeeded. IMO, having the getter method helps the reader
easier to understand and future maintainence.
>> line 564-567: this comment should belong to loadInitialDrivers right?
> I had left it there as both methods kind of flow together, but I moved
> it per your suggestion. Thank you
>> Nit line 567 - not align (one more space to the right needed)
> fixed
>> Is driversSync object necessary?
> I am not 100% sure but I thought having its own monitor would further
> reduce the contention possibility and given this was only reported in
> this one case, I was playing it safe :-).
>
>> Can you make loadInitialDrivers be
>> a synchronized method and move line 575-581 except 578 to the
>> loadInitialDrivers method (driversInitialized is volatile which is
>> good).
> That is doable, I just thought keeping the DCL code together makes it
> easier to read and decided to have its own monitor. If you feel we
> should go this way, I will, just let me know
>
The lock is to make sure the drivers are loaded once. AFAICT, there is
no other method locking DriverManager class except getConnection and I
don't see any issue.
I forgot to include this comment in my previous reply:
/*
* When callerCl is null, we should check the application's
* (which is invoking this class indirectly)
* classloader, so that the JDBC driver class outside rt.jar
* can be loaded from here.
*/
ClassLoader callerCL = caller != null ? caller.getClassLoader() : null;
synchronized(DriverManager.class) {
// synchronize loading of the correct classloader.
if (callerCL == null) {
callerCL = Thread.currentThread().getContextClassLoader();
}
}
I don't understand why this needs to synchronize in order to get TCCL.
You are getting the TCCL of the current thread and callerCL is a local
variable. So I think this can be removed.
Mandy
More information about the core-libs-dev
mailing list