RFR 8060068 : Remove the static initializer block from DriverManager

Lance Andersen lance.andersen at oracle.com
Tue Dec 2 20:30:16 UTC 2014


Hi Mandy,

Thank you for the review, please see below

On Dec 2, 2014, at 2:56 PM, Mandy Chung <mandy.chung at oracle.com> wrote:

> On 12/1/14 8:52 AM, Lance Andersen wrote:
>> Hi all,
>> 
>> Looking for a review for this change to DriverManager to reduce the possibility on a deadlock on <clinit>. that I have been discussing with Mandy.
>> 
>> 
>> The change removes the static initializer block as well as the synchronized keyword from registerDriver.
>> 
>> The webrev can be found at http://cr.openjdk.java.net/%7Elancea/8060068/webrev.02/
>> 
> initDriversIfNeeded() is called to ensure that the drivers are registered. 
This is a one time operation  only those specified via the system property of supporting ServiceLoader.  

A user could invoke Class.forName(<my java.sql.Driver impl>) and cause a driver to be registered at any time
> 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 :-)
> 
> 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 

Best,
Lance
> 
> Mandy



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com






More information about the core-libs-dev mailing list