RFR [JDK8]: 7169894: JAXP Plugability Layer: using service loader

[Paul Sandoz]

Hi Joe,

It looks a good start.

There is duplication in the 6 factory finding classes, can some of it be consolidated in one shared factory helper class?

Taking javax.xml.datatyoe.FactoryFinder as an example:

When iterating over the service instances you are catching ConfigurationError 

 264         ServiceLoader loader = ServiceLoader.load(serviceClass, cl);
 265         final Iterator providers = loader.iterator();
 266         while (providers.hasNext()) {
 267             try {
 268                 Object provider = providers.next();
 269                 if (provider.getClass().getName().contains(fallbackClassName)) {
 270                     defaultProvider = provider;
 271                 } else {
 272                     return provider;
 273                 }
 274             } catch (ConfigurationError e) {
 275                 // This can happen because of class loader mismatch or any other reason.
 276                 // log and continue to next one
 277                 if (debug) {
 278                     dPrint("The provider can not be instantiated due to: " + e + ".");
 279                 }
 280             }
 281         }
Did you mean ServiceConfigurationError?

IIUC the previous code parsed a META-INF/services file and picked the first entry if present and attempt to instantiate that. 

I gather the approach you want to achieve with ServiceLoader is to "keep on trucking". In addition if nothing but an instance of the default service provider class is obtained then use that. From what i can tell the "fallbackClassName" parameter is a fully qualified class name so you need to do getName().equals(fallbackClassName).

An alternative approach is to always assume that the default service provider class is never registered in META-INF/services or in module declaration. It should simplify things.

You are first trying to use ServiceLoader with the thread context class loader, then if that fails using the system class loader (since FactoryFinder.class.getCLassLoader() == null).

 234     private static Object findServiceProvider(String factoryId, String fallbackClassName)
 235         throws ConfigurationError
 236     {
 237         final Class<?> serviceClass;
 238         try {
 239             serviceClass = Class.forName(factoryId);
 240         } catch (ClassNotFoundException e) {
 241             throw new ConfigurationError("Unable to load " + factoryId, e);
 242         }
 243 
 244         Object provider = null;        
 245         
 246         // First try the Context ClassLoader
 247         ClassLoader cl = ss.getContextClassLoader();        
 248         if (cl != null) {
 249             provider = findProvider(serviceClass, cl, fallbackClassName);
 250             if (provider != null)  return provider;
 251         }
 252         
 253         // If no provider found then try the current ClassLoader
 254         provider = findProvider(serviceClass, FactoryFinder.class.getClassLoader(), fallbackClassName);
 255         if (provider != null)  return provider;
 256 
 257         return null;
 258     }
I am wondering if we can just get away with using ServiceLoader.load(serviceClass), possibly not given the current implemented (but not documented) behavior. In any case we should document the class loaders being used, and in what order, with ServiceLoader.

I am not so sure about the "keep on trucking" approach when iterating over service instances. The service mechanism is being used to register a factory class that is a service provider class. If the first item in the service instance iterator cannot be instantiated it signals a configuration error.

--

I think the use of ServiceLoader by JAXP is really good input to improving ServiceLoader e.g.:

  ServiceLoader.withDefault(defaultServiceProviderClass).load(serviceInterface);

or

  MyService serviceInstance = ServiceLoader.withDefault(defaultServiceProviderClass).
          first(serviceInterfaceClass);

You could emulate this with your own shared factory helper class.

Paul.



On Jun 21, 2012, at 9:55 AM, Joe Wang wrote:

> Hi,
> 
> This is a patch to replace the manual process in the 3rd step of the JAXP implementation resolution mechanism with ServiceLoader.  Please see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7169894 for more details about the issue.
> 
> Note that FactoryFinder is duplicated for each JAXP subpackage. The ones in the following packages are the same:
> /jaxp-api/src/javax/xml/datatype/FactoryFinder.java
> /jaxp-api/src/javax/xml/parsers/FactoryFinder.java
> /jaxp-api/src/javax/xml/stream/FactoryFinder.java
> /jaxp-api/src/javax/xml/transform/FactoryFinder.java
> 
> The following two are similar except that they perform Schema Language or Object Model support check:
> /jaxp-api/src/javax/xml/validation/SchemaFactoryFinder.java
> /jaxp-api/src/javax/xml/xpath/XPathFactoryFinder.java
> 
> It's a bit rush since I have only had time to test regular JDK using JDK 1.6.0_27.  Further test on jigsaw is needed.
> All jaxp unit/SQE tests are passed. But TCK test is still running.  So please take this as an initial review.
> 
> webrev:
> http://cr.openjdk.java.net/~joehw/jdk8/7169894/webrev/
> 
> Thanks,
> Joe
> 
>