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