Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-03 Thread Lance Andersen
On Dec 3, 2014, at 11:57 AM, Mandy Chung wrote: > > On 12/3/2014 8:18 AM, Lance Andersen wrote: >> >> Thank you Sean. As this code path is only called 1 time, i am not concerned >> that performance will be an issue. If you and Mandy prefer me to remove >> it, I can, just let me know. >>

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-03 Thread Mandy Chung
On 12/3/2014 8:18 AM, Lance Andersen wrote: Thank you Sean. As this code path is only called 1 time, i am not concerned that performance will be an issue. If you and Mandy prefer me to remove it, I can, just let me know. Yes, I agree it is narrow. The suggestion to add the limited doPr

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-03 Thread Lance Andersen
On Dec 3, 2014, at 10:39 AM, Sean Mullan wrote: > On 12/03/2014 10:03 AM, Lance Andersen wrote: Note, I also tweaked the doPriviliged block for the JDBC property >>> > >>> >It's nice to see use of limited doPrivileged. Limited doPrivileged >>> >restricts the permissions be accessed by t

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-03 Thread Sean Mullan
On 12/03/2014 10:03 AM, Lance Andersen wrote: Note, I also tweaked the doPriviliged block for the JDBC property > >It's nice to see use of limited doPrivileged. Limited doPrivileged restricts the permissions be accessed by the doPrivileged block. On the other hand, since it only calls Syst

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-03 Thread Lance Andersen
Hi Bernd, On Dec 2, 2014, at 6:33 PM, Bernd Eckenfels wrote: > Hello Mandy and Lance, > > (sorry, not a full quote for more focused answers, inline) > > Am Tue, 02 Dec 2014 14:10:06 -0800 > schrieb Mandy Chung : > >> Would you be able to try this patch and see if the deadlocks are >> reproduc

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-03 Thread Lance Andersen
Hi Mandy, On Dec 2, 2014, at 11:28 PM, Mandy Chung wrote: > > On 12/2/2014 1:47 PM, Lance Andersen wrote: >> The revised webrev is here >> http://cr.openjdk.java.net/~lancea/8060068/webrev.03/ >> > > Looks good. Nit: line 443 and a few places in the getConnection need a > space in "for(",

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Mandy Chung
On 12/2/2014 1:47 PM, Lance Andersen wrote: The revised webrev is here http://cr.openjdk.java.net/~lancea/8060068/webrev.03/ Looks good. Nit: line 443 and a few places in the getConnection need a space in "for(", "if(" Note, I

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Bernd Eckenfels
Hello Mandy and Lance, (sorry, not a full quote for more focused answers, inline) Am Tue, 02 Dec 2014 14:10:06 -0800 schrieb Mandy Chung : > Would you be able to try this patch and see if the deadlocks are > reproducible? Lance has been trying to get customers to verify this > patch as this c

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Lance Andersen
Hi Mandy, On Dec 2, 2014, at 5:10 PM, Mandy Chung wrote: > Hi Bernd, > > On 12/2/14 1:49 PM, Bernd Eckenfels wrote: >> Hello, >> >> 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

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Lance Andersen
Bernd, Thank you for your input On Dec 2, 2014, at 4:49 PM, Bernd Eckenfels wrote: > Hello, > > 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

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Mandy Chung
Hi Bernd, On 12/2/14 1:49 PM, Bernd Eckenfels wrote: Hello, 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

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Bernd Eckenfels
Hello, 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 w

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Lance Andersen
On Dec 2, 2014, at 4:12 PM, Mandy Chung wrote: > 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

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Mandy Chung
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 : r

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Lance Andersen
Hi Mandy, Thank you for the review, please see below On Dec 2, 2014, at 2:56 PM, Mandy Chung 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 . that I have been discussing wi

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Mandy Chung
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 . that I have been discussing with Mandy. The change removes the static initializer block as well as the synchronized keyword from registerDriver

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Lance Andersen
Hi Stuart, On Dec 2, 2014, at 1:35 PM, Stuart Marks wrote: > Hi Lance, > > Overall, looks fine. Thank you for the review > > Typo "earleir" at line 569. fixed > > I agree with having two separate init methods, since initDriversIfNeeded() > conveniently separates the (safe) double-checked lo

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-02 Thread Stuart Marks
Hi Lance, Overall, looks fine. Typo "earleir" at line 569. I agree with having two separate init methods, since initDriversIfNeeded() conveniently separates the (safe) double-checked locking idiom from the actual initialization legwork in loadInitialDrivers(). I'm not entirely convinced, ho

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-01 Thread Lance Andersen
Hi Ulf, thank you for the input and suggestion On Dec 1, 2014, at 3:27 PM, Ulf Zibis wrote: > Hi Lance, > > to me it's irritating, why there are 2 methods: > - initDriversIfNeeded() > - loadInitialDrivers() > I would combine both to one method. Mandy had asked me previously about this and h

Re: RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-01 Thread Ulf Zibis
Hi Lance, to me it's irritating, why there are 2 methods: - initDriversIfNeeded() - loadInitialDrivers() I would combine both to one method. In lines 90 + 92 there are double spaces. -Ulf Am 01.12.2014 um 17:52 schrieb Lance Andersen: Hi all, Looking for a review for this change to DriverMan

RFR 8060068 : Remove the static initializer block from DriverManager

2014-12-01 Thread Lance Andersen
Hi all, Looking for a review for this change to DriverManager to reduce the possibility on a deadlock on . 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.ope