hg: jdk7/tl/jdk: 6843995: RowSet 1.1 updates

[David Schlosnagle]

On Fri, Nov 5, 2010 at 5:51 PM, Lance Andersen - Oracle
<Lance.Andersen at oracle.com> wrote:
>
> Hi Remi (and team),
> I made changes to SyncFactory and one other class for a similar error.  Also cleaned up a couple of other minor issues in these classes.
> The webrev can be found at http://cr.openjdk.java.net/~lancea/6982530/
> Thank you for catching the error.
> Regards
> Lance

Lance,

I had a few questions and comments:

In JdbcRowSetResourceBundle.java - private static String fileName is
unused, can this be removed? My only hesitation is that the class is
Serializable, and although the field is a static, the specification
<http://download.oracle.com/javase/6/docs/platform/serialization/spec/version.html#6678>
lists "Deleting fields" as an incompatible change without qualifying
whether this implies deleting static fields is serial version
incompatible (I wouldn't think it should be incompatible as statics
aren't serialized by default, but I'm not a serialization expert by
any means).

In SyncFactory.java - the following fields are unused, can these be removed?
    private static String default_provider
    private static Level rsLevel
    private static Object logSync
    private static java.io.PrintWriter logWriter

In SyncFactory.initJNDIContext(), isn't there still a potential
visibility problem with reads of static Context field ic because the
write is done under the class' monitor (in setJNDIContext), while the
read is not? This could lead to a race condition which would possibly
allow duplicate initialization under concurrent calls. Also, the
static boolean field lazyJNDICtxRefresh does not seem to have any
visibility guarantees, leading to similar problems. To fix this,
should initJNDIContext() be synchronized?

In SyncFactory.java, this is more stylistic, but I'm curious why a
volatile double-checked lock was chosen over the (cleaner IMHO) lazy
initialize-on-demand idiom described in Java Concurrency in Practice
[Brian Goetz et al., 2006 p348]. The tradeoff I see is a mutable
static volatile field versus the overhead of an additional class file.
I'm just curious as I've been cleaning up a lot of unsafe
double-checked locks in some code at work and I'd like to understand
the rationale better from the core-libs perspective. I'd envisaged
something along these lines with the removal of the private static
volatile SyncFactory syncFactory:

     /**
      * Returns the {@code SyncFactory} singleton.
      *
      * @return the {@code SyncFactory} instance
      */
     public static SyncFactory getSyncFactory() {
         // This method uses the thread safe lazy initialization holder class
         // idiom [Brian Goetz et al. Java Concurrency in Practice, 2006 p348]
         return Singleton.syncFactory;
     }

     private static class Singleton {
         static final SyncFactory syncFactory = new SyncFactory();
     }

Thanks,
Dave