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

[Joe Wang]

On 8/31/2012 4:20 AM, Alan Bateman wrote:
> On 31/08/2012 08:59, Joe Wang wrote:
>>
>> Yes, it works just fine under normal conditions.
>>
>> Note the old process: read the service file using TCCL (if null, 
>> system cl), if nothing found, then BCL
>>
>> Somehow I was under the impression ServiceLoader does just that so 
>> that we could delegate to it.  It's probably a good thing that it 
>> actually doesn't. If it did, it'd cause error in the following case:
>>
>>    The TCCL's parent is BCL  (that is Bootstrap), a jaxp impl (e.g. 
>> Xerces) is on the classpath
>>     Old process: try TCCL, not found, try BCL, not found
>>     ServiceLoader: use TCCL, not found, return null  (I thought it'd 
>> continue using System CL, but it didn't)
>>
>>     **although worked differently, the results are the same, so not 
>> an issue in terms of the result, still a behavior change though.
>>
>> I did many tests, but so far the results are the same before and 
>> after the patch.
>>
>> If TCCL is set to null, I actually saw SCE other than SchemaFactory, 
>> for example:
>> javax.xml.stream.FactoryConfigurationError: 
>> javax.xml.stream.XMLEventFactory: Provider 
>> org.apache.xerces.stax.XMLEventFactoryImpl not found
>>
>> --Joe
> This is a mess. I realize it pre-dates your watch but it looks like 
> we're going to have to sort this out in order to make progress. I 
> think sorting this out will require some behavior changes so that the 
> factories are located in a consistent manner and also work 
> consistently when there is a security manager set.
>
> So what would you think about putting the ServiceLoader changes on 
> hold for a little bit and spending some time to get the factory 
> finders working consistently. I hope this will be mostly just fixing 
> the SecuritySupport.getContextClassLoader implementations and so will 
> not conflict with the SL work. As part of this then we get a list of 
> existing vs. proposal behavior for each factory and for each of the 
> security manager + no security manager cases. This will allow us to 
> assess the impact to each factory finder. My gut feel is that the 
> impact will not be too significant and in most cases will be an 
> improvement to how it works today.
>
> I also agree with Paul about getting a useful set of tests as it's 
> impossible to say from reading the javadoc how it should behave. These 
> tests will help you to get the list of current behavior, proposed 
> behavior and should give you confidence when you get to the changing 
> this code to use ServiceLoader.

Okay,  let's streamline the factories.  I think we should:
1) align to the SAX/DOM since these are most frequently used, and 
proven.  Any changes to other factories will have lower impact in 
comparison;

2) no spec change involved:  for example, the StAX factories have 
newFactory/newInstance(String factoryId, ClassLoader classLoader), while 
others define the first parameter as factory classname, e.g. 
newInstance(String factoryClassName, ClassLoader classLoader).  We also 
know that some factories have throws clause while others don't.  In this 
effort of making factories behave consistently, we'll restrain from 
making any spec change.

--Joe

>
> -Alan