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 mandy.ch...@oracle.com 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

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 e...@zusammenkunft.net 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 mandy.ch...@oracle.com: Would you be able to try this patch and

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

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 sean.mul...@oracle.com 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

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

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 mandy.ch...@oracle.com 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

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,

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 stuart.ma...@oracle.com 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)

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

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 mandy.ch...@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

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 :

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 mandy.ch...@oracle.com 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

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

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 Lance Andersen
Bernd, Thank you for your input On Dec 2, 2014, at 4:49 PM, Bernd Eckenfels e...@zusammenkunft.net 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

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 mandy.ch...@oracle.com 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

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 mandy.ch...@oracle.com: Would you be able to try this patch and see if the deadlocks are reproducible? Lance has been trying to get customers to verify

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/ http://cr.openjdk.java.net/%7Elancea/8060068/webrev.03/ Looks good. Nit: line 443 and a few places in the getConnection need a space in for(, if( Note, I also

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 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

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

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 ulf.zi...@cosoco.de 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