6982530: javadoc update to SyncFactory & JdbcResource bundle for synchronization issues

Reviewed-by: alanb

jdk/src/share/classes/com/sun/rowset/JdbcRowSetResourceBundle.java

@@ -27,7 +27,6 @@ package com.sun.rowset;
 
 import java.io.*;
 import java.util.*;
-import java.lang.*;
 
 /**
  * This class is used to help in localization of resources,
@@ -42,28 +41,28 @@ public class JdbcRowSetResourceBundle implements Serializable {
      * This <code>String</code> variable stores the location
      * of the resource bundle location.
      */
-    static String fileName;
+    private static String fileName;
 
     /**
      * This variable will hold the <code>PropertyResourceBundle</code>
      * of the text to be internationalized.
      */
-    transient PropertyResourceBundle propResBundle;
+    private transient PropertyResourceBundle propResBundle;
 
     /**
      * The constructor initializes to this object
      *
      */
-    static JdbcRowSetResourceBundle jpResBundle;
+    private static volatile JdbcRowSetResourceBundle jpResBundle;
 
     /**
-     * The varible which will represent the properties
+     * The variable which will represent the properties
      * the suffix or extension of the resource bundle.
      **/
     private static final String PROPERTIES = "properties";
 
     /**
-     * The varibale to represent underscore
+     * The variable to represent underscore
      **/
     private static final String UNDERSCORE = "_";
 

jdk/src/share/classes/javax/sql/rowset/spi/SyncFactory.java

@@ -197,12 +197,6 @@ import javax.naming.*;
  */
 public class SyncFactory {
 
-    /*
-     * The variable that represents the singleton instance
-     * of the <code>SyncFactory</code> class.
-     */
-    private static SyncFactory syncFactory = null;
-
     /**
      * Creates a new <code>SyncFactory</code> object, which is the singleton
      * instance.
@@ -252,7 +246,7 @@ public class SyncFactory {
     /**
      * The <code>Logger</code> object to be used by the <code>SyncFactory</code>.
      */
-    private static Logger rsLogger;
+    private static volatile Logger rsLogger;
     /**
      *
      */
@@ -315,27 +309,12 @@ public class SyncFactory {
      * @return the <code>SyncFactory</code> instance
      */
     public static SyncFactory getSyncFactory() {
-
-        // This method uses the Singleton Design Pattern
-        // with Double-Checked Locking Pattern for
-        // 1. Creating single instance of the SyncFactory
-        // 2. Make the class thread safe, so that at one time
-        //    only one thread enters the synchronized block
-        //    to instantiate.
-
-        // if syncFactory object is already there
-        // don't go into synchronized block and return
-        // that object.
-        // else go into synchronized block
-
-        if (syncFactory == null) {
-            synchronized (SyncFactory.class) {
-                if (syncFactory == null) {
-                    syncFactory = new SyncFactory();
-                } //end if
-            } //end synchronized block
-        } //end if
-        return syncFactory;
+        /*
+         * Using Initialization on Demand Holder idiom as
+         * Effective Java 2nd Edition,ITEM 71, indicates it is more performant
+         * than the Double-Check Locking idiom.
+         */
+        return SyncFactoryHolder.factory;
     }
 
     /**
@@ -435,11 +414,7 @@ public class SyncFactory {
             }
         }
     }
-    /**
-     * The internal boolean switch that indicates whether a JNDI
-     * context has been established or not.
-     */
-    private static boolean jndiCtxEstablished = false;
+
     /**
      * The internal debug switch.
      */
@@ -629,6 +604,10 @@ public class SyncFactory {
         if (sec != null) {
             sec.checkPermission(SET_SYNCFACTORY_PERMISSION);
         }
+
+        if(logger == null){
+            throw new NullPointerException("You must provide a Logger");
+        }
         rsLogger = logger;
     }
 
@@ -663,8 +642,12 @@ public class SyncFactory {
         if (sec != null) {
             sec.checkPermission(SET_SYNCFACTORY_PERMISSION);
         }
+
+        if(logger == null){
+            throw new NullPointerException("You must provide a Logger");
+        }
+        logger.setLevel(level);
         rsLogger = logger;
-        rsLogger.setLevel(level);
     }
 
     /**
@@ -674,11 +657,14 @@ public class SyncFactory {
      * @throws SyncFactoryException if no logging object has been set.
      */
     public static Logger getLogger() throws SyncFactoryException {
+
+        Logger result = rsLogger;
         // only one logger per session
-        if (rsLogger == null) {
+        if (result == null) {
             throw new SyncFactoryException("(SyncFactory) : No logger has been set");
         }
-        return rsLogger;
+
+        return result;
     }
 
     /**
@@ -699,7 +685,7 @@ public class SyncFactory {
      *  {@code checkPermission} method denies calling {@code setJNDIContext}
      * @see SecurityManager#checkPermission
      */
-    public static void setJNDIContext(javax.naming.Context ctx)
+    public static synchronized void setJNDIContext(javax.naming.Context ctx)
             throws SyncFactoryException {
         SecurityManager sec = System.getSecurityManager();
         if (sec != null) {
@@ -709,17 +695,16 @@ public class SyncFactory {
             throw new SyncFactoryException("Invalid JNDI context supplied");
         }
         ic = ctx;
-        jndiCtxEstablished = true;
     }
 
     /**
-     * Controls JNDI context intialization.
+     * Controls JNDI context initialization.
      *
      * @throws SyncFactoryException if an error occurs parsing the JNDI context
      */
-    private static void initJNDIContext() throws SyncFactoryException {
+    private static synchronized void initJNDIContext() throws SyncFactoryException {
 
-        if (jndiCtxEstablished && (ic != null) && (lazyJNDICtxRefresh == false)) {
+        if ((ic != null) && (lazyJNDICtxRefresh == false)) {
             try {
                 parseProperties(parseJNDIContext());
                 lazyJNDICtxRefresh = true; // touch JNDI namespace once.
@@ -793,6 +778,13 @@ public class SyncFactory {
             enumerateBindings(bindings, properties);
         }
     }
+
+    /**
+     * Lazy initialization Holder class used by {@code getSyncFactory}
+     */
+    private static class SyncFactoryHolder {
+        static final SyncFactory factory = new SyncFactory();
+    }
 }
 
 /**