RFR: javax.xml.transform: Using ServiceLoader to load JAXP stream factories (7169894: JAXP Plugability Layer: using service loader) (original) (raw)

Joe Wang huizhe.wang at oracle.com
Wed Dec 19 18:56:28 UTC 2012

On 12/19/2012 6:34 AM, Daniel Fuchs wrote:

Hi,

Please find below an updated webrev for the javax.xml.transform package, taking into account Mandy's & Joe's comments - namely: 1. Fixed call to creationMethod.invoke. 2. Got rid of the 4 args FactoryFinder.newInstance method. 3. Got rid of the useBSClsLoader which was always passed as 'false'. (thanks Mandy!)

I agree with the change. Just to clarify, the only case where useBSClsLoader could be 'true' was in the old findJarServiceProvider method.

There might be subtle things the ServiceLoader does that could be different from the findJarServiceProvider process. In both processes, ContextClassLoader is used first, then the old process would try the classloader of the finder class which is the bootclassloader.

The ServiceLoader spec says: The ServiceLoader.load(service) is equivalent to ServiceLoader.load(service, Thread.currentThread().getContextClassLoader()) where "loader - The class loader to be used to load provider-configuration files and provider classes, or null if the system class loader (or, failing that, the bootstrap class loader) is to be used "

So we need to make sure it works fine when ContextClassLoader is null, or ContextClassLoader's parent is not the app class loader. I think we have regression tests for the scenarios. Since you've run them, I don't see there's any problem.

-Joe

<http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.transform/webrev.01/>

-- daniel On 12/18/12 11:02 PM, Daniel Fuchs wrote: On 12/18/12 9:06 PM, Mandy Chung wrote: In FactoryFinder.newInstanceNoServiceLoader method, L223, 226 - NoSuchMethodException will be thrown if such method doesn't exist. creationMethod will not be null. Thanks - yes - you're right of course - no need to check for null... L236 - this change is not needed, right? The method is a static no-arg method. You passed an additional argument creationMethod as the first parameter although it's harmless as it's ignored. Oops - my bad. That's a mistake - I did too many successive changes - should be creationMethod.invoke(null) of course. And my tests didn't even catch it!

A minor comment: 151 static T newInstance(Class type, String className, ClassLoader cl, boolean doFallback) 152 throws TransformerFactoryConfigurationError 153 { 154 return newInstance(type, className, cl, doFallback, false, false); 155 } The FactoryFinder.newInstance method 4-argument version is only called by TransformerFactory.newInstance(String factoryClassName, ClassLoader classLoader). Perhaps you can clean this up TransformerFactory to call the Factory.newInstance method 6-argument version. 3 successive boolean parameters... I hate that ;-) Yes I think I can do this cleanup... Thanks Mandy, -- daniel

Thanks Mandy

More information about the core-libs-dev mailing list