Re: Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();

2012-08-12 Thread David Holmes 

Lance,

On 11/08/2012 3:19 AM, Lance Andersen - Oracle wrote:

Looking for a reviewer for the following change:

-  add  a call to Thread.currentThread().getContextClassLoader() to 
DriverManager.getDriver()
- Remove the synchronized block for the same call in getConnection()


What is the context for the change? What is it trying to achieve or avoid?

David


Thank you.

Best
Lance

localhost:sql lanceandersen$ hg diff DriverManager.java
diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
--- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14 
2012 -0400
+++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00 
2012 -0400
@@ -264,6 +264,10 @@
  // be null.
  ClassLoader callerCL = DriverManager.getCallerClassLoader();

+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
+}
+
  // Walk through the loaded registeredDrivers attempting to locate 
someone
  // who understands the given URL.
  for (DriverInfo aDriver : registeredDrivers) {
@@ -563,11 +567,8 @@
   * classloader, so that the JDBC driver class outside rt.jar
   * can be loaded from here.
   */
-synchronized(DriverManager.class) {
-  // synchronize loading of the correct classloader.
-  if(callerCL == null) {
-  callerCL = Thread.currentThread().getContextClassLoader();
-   }
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
  }

  if(url == null) {
localhost:sql lanceandersen$

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

Re: Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();

2012-08-11 Thread Lance Andersen - Oracle 
For completeness, they could be added and I guess I can do it as part of this 
particular change.

I would probably move the call to  DriverManager.getCallerClassLoader() to 
avoid the duplication of the if block.

Best
Lance
On Aug 10, 2012, at 5:02 PM, David Schlosnagle wrote:

 Lance,
 
 There are a couple of other uses of
 DriverManager.getCallerClassLoader() that do no fall back to
 Thread.currentThread().getContextClassLoader() if the caller class
 loader is null (bootstrap loader). Should these be updated as well?
 
 From 
 http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/sql/DriverManager.java:
  Line 334 in deregisterDriver(Driver driver)
  Line 365 in getDrivers()
 
 Thanks,
 Dave
 
 
 On Fri, Aug 10, 2012 at 1:19 PM, Lance Andersen - Oracle
 lance.ander...@oracle.com wrote:
 Looking for a reviewer for the following change:
 
 -  add  a call to Thread.currentThread().getContextClassLoader() to 
 DriverManager.getDriver()
 - Remove the synchronized block for the same call in getConnection()
 
 Thank you.
 
 Best
 Lance
 
 localhost:sql lanceandersen$ hg diff DriverManager.java
 diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
 --- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14 
 2012 -0400
 +++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00 
 2012 -0400
 @@ -264,6 +264,10 @@
 // be null.
 ClassLoader callerCL = DriverManager.getCallerClassLoader();
 
 +if(callerCL == null) {
 +callerCL = Thread.currentThread().getContextClassLoader();
 +}
 +
 // Walk through the loaded registeredDrivers attempting to locate 
 someone
 // who understands the given URL.
 for (DriverInfo aDriver : registeredDrivers) {
 @@ -563,11 +567,8 @@
  * classloader, so that the JDBC driver class outside rt.jar
  * can be loaded from here.
  */
 -synchronized(DriverManager.class) {
 -  // synchronize loading of the correct classloader.
 -  if(callerCL == null) {
 -  callerCL = Thread.currentThread().getContextClassLoader();
 -   }
 +if(callerCL == null) {
 +callerCL = Thread.currentThread().getContextClassLoader();
 }
 
 if(url == null) {
 localhost:sql lanceandersen$
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 


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

Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();

2012-08-10 Thread Lance Andersen - Oracle 
Looking for a reviewer for the following change:

-  add  a call to Thread.currentThread().getContextClassLoader() to 
DriverManager.getDriver()
- Remove the synchronized block for the same call in getConnection()

Thank you.

Best
Lance

localhost:sql lanceandersen$ hg diff DriverManager.java
diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
--- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14 
2012 -0400
+++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00 
2012 -0400
@@ -264,6 +264,10 @@
 // be null.
 ClassLoader callerCL = DriverManager.getCallerClassLoader();
 
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
+}
+
 // Walk through the loaded registeredDrivers attempting to locate 
someone
 // who understands the given URL.
 for (DriverInfo aDriver : registeredDrivers) {
@@ -563,11 +567,8 @@
  * classloader, so that the JDBC driver class outside rt.jar
  * can be loaded from here.
  */
-synchronized(DriverManager.class) {
-  // synchronize loading of the correct classloader.
-  if(callerCL == null) {
-  callerCL = Thread.currentThread().getContextClassLoader();
-   }
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
 }
 
 if(url == null) {
localhost:sql lanceandersen$ 

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

Re: Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();

2012-08-10 Thread David Schlosnagle 
Lance,

There are a couple of other uses of
DriverManager.getCallerClassLoader() that do no fall back to
Thread.currentThread().getContextClassLoader() if the caller class
loader is null (bootstrap loader). Should these be updated as well?

From 
http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/sql/DriverManager.java:
  Line 334 in deregisterDriver(Driver driver)
  Line 365 in getDrivers()

Thanks,
Dave


On Fri, Aug 10, 2012 at 1:19 PM, Lance Andersen - Oracle
lance.ander...@oracle.com wrote:
 Looking for a reviewer for the following change:

 -  add  a call to Thread.currentThread().getContextClassLoader() to 
 DriverManager.getDriver()
 - Remove the synchronized block for the same call in getConnection()

 Thank you.

 Best
 Lance

 localhost:sql lanceandersen$ hg diff DriverManager.java
 diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
 --- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14 
 2012 -0400
 +++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00 
 2012 -0400
 @@ -264,6 +264,10 @@
  // be null.
  ClassLoader callerCL = DriverManager.getCallerClassLoader();

 +if(callerCL == null) {
 +callerCL = Thread.currentThread().getContextClassLoader();
 +}
 +
  // Walk through the loaded registeredDrivers attempting to locate 
 someone
  // who understands the given URL.
  for (DriverInfo aDriver : registeredDrivers) {
 @@ -563,11 +567,8 @@
   * classloader, so that the JDBC driver class outside rt.jar
   * can be loaded from here.
   */
 -synchronized(DriverManager.class) {
 -  // synchronize loading of the correct classloader.
 -  if(callerCL == null) {
 -  callerCL = Thread.currentThread().getContextClassLoader();
 -   }
 +if(callerCL == null) {
 +callerCL = Thread.currentThread().getContextClassLoader();
  }

  if(url == null) {
 localhost:sql lanceandersen$

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