RFR 8060068 : Remove the static initializer block from DriverManager

Bernd Eckenfels ecki at zusammenkunft.net
Tue Dec 2 21:49:27 UTC 2014


just want to add two somewhat related observations:

we have a virtual JDBC driver which maps back to an real driver (or to
an Pool/Datasource which uses a real driver. This is to allow
JDBC URLs to work in different environments with no config. (Thats is
not the nices solution, but after we resolved the deadlocks it
works stable with the legacy code we need to support).

This does have two consequences which are related to this patch:

a) #getConnection() is used quite often, as it tunnels through to a
high performing pool (already mentioned as a good reason for DCL).

b) there have been some deadlock conditions in the past in this area. I
can try to find the details later on, but it involved class
loading/registration and the driver's synchronized.

So it is a bit risky (assuming we are not the only ones using the SPI
in this creative way) to play with the locking.

At the same time, it is also needed. Maybe the more finegrained lock
and the lock-free happy path help with it. I wonder, is there a
special procedure to warn in the release notes/compatibility guide
other than having the patch listed in a changelog? In order to warn
people switching to 9 (in 5 years or so :).


 Am Tue, 2 Dec
2014 15:30:16 -0500 schrieb Lance Andersen <lance.andersen at oracle.com>:

> 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