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

[Joe Wang]

On 6/21/2012 11:04 AM, Alan Bateman wrote:
> On 21/06/2012 18:34, Joe Wang wrote:
>>
>> :
>>>
>>> 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?
>>
>> ConfigurationError is internally defined. It's a contract between the 
>> Factory and FactoryFinder classes.  It seems to me it was a result of 
>> sharing the FactoryFinders and different exception types defined in 
>> the spec for different factories.
> With ServicLoader then ServiceConfigurationError is thrown if there is 
> a problem reading the service configuration file or the concrete type 
> is not a sub-type or it cannot be instantiated.
>
> With JAXP then I think most of the places it is specified to throw a 
> JAXP <something>ConfigurationException when the provider cannot be 
> instantiated (it seems to be silent as to issues encountered reading 
> the service configuration file).
>
> Minimally the above code will need to be changed to catch 
> ServiceConfigurationError. Whether it ignores it and attempts to 
> continue, or throws a JAXP ConfigurationError needs needs 
> consideration. The existing behavior seems to be to ignore 
> IOExceptions if there is a problem reading the service configuration 
> file but I don't think it can distinguish this case when it gets a 
> SCE. Therefore the safest thing may be to just throw 
> ConfiguarationError and the callers of the factory finder call will 
> translate it into the appropriate (and specified) JAXP 
> *ConfigurationException.

Right, that's a mistake. It was SCE, somehow I managed to converted all 
to the original ConfigurationError.

The current sequence is:

 1.         if service loader fails to load a provider, i.e. a SCE, the
    process continue to try the next provider if exists  -- it would
    have been a solution for 697514. A SEC would be thrown when an
    improper provider is encountered.
 2.         if default provider is found, returns it only if there's no
    other providers
 3.         if no provider found, return and the jaxp impl resolution
    process continues
 4.         if no provider found after the whole process, report a
    xxxConfigurationException. The xxxConfigurationException can not
    therefore take the SCE in (1) as its cause.

(4) almost never happens in regular jdk, but it may in jigsaw.
(2) as what I did now, maybe incorrect, since the app classloader can 
actually load the jaxp default impl placed in the endorsed dir or 
bootclasspath.  This change therefore would be incorrect for regular 
jdk. However, it is intended when the ri becomes a default module in jigsaw.

The original spec did not classify which classloader will be used. 
People learned that how it worked. I felt the same thing now with 
ServiceLoader. Now that it seems to me the 'findServiceProvider' process 
is dictated by the ServiceLoader.  What I found was that it aggregated 
and delivered back all of the providers whether they are on classpath or 
bootclasspath or endorsed dir.

We're not considering this change for jdk7 or older, i.e. I haven't 
thought about fixing 6975142.  For jdk8, it seems we have to live with 
the way it is in (2), a seemly incompatible behavior in backward 
compatible mode, although,  it is a very rare case (I regret to have 
taught or sent standalone jar files to those customers :)  ).

-Joe


>
> -Alan.
>
>
>