CFR: javax.xml.parsers: Using ServiceLoader to load JAXP parser factories (7169894: JAXP Plugability Layer: using service loader) (original) (raw)

Alan Bateman Alan.Bateman at oracle.com
Tue Dec 4 21:12:57 UTC 2012

On 04/12/2012 14:42, Daniel Fuchs wrote:

:

The class description also suggests that a SCE will be wrapped as a FactoryConfigurationException but in FactoryFinder I see that it actual wraps a RuntimeException. I think you can just use the no-arg constructor then then use initCause to set the cause to the SCE. I have refrained to do that because I think there might be code that brutally does things like (Exception) error.getCause(); (I've seen this kind of pattern in the JAXP code - although I don't remember exactly where...) The reason is that FactoryConfigurationError is for some reason obviously supposed to wrap only exceptions - as its constructor will take only exceptions - and not throwables, and then overrides getCause() to return a locally cached variable of type Exception - see <http://hg.openjdk.java.net/jdk8/tl/jaxp/raw-file/tip/src/javax/xml/parsers/FactoryConfigurationError.java> Using initCause() would therefore have no effect, unless the implementation of javax.xml.parsers.FactoryConfigurationError is changed to accept Throwable as cause - which could - I think have undesirable side effects on existing code. My guess is that this is an oddity inherited from the time when Cause was not defined at Throwable level - but managed locally on specific exceptions classes instead. I am fearing that changing the implementation of getCause() to allow setting the SCE directly as the cause might break compatibility, causing some ClassCastException to be thrown here and there. I see your point and happy to see that you are very careful with these changes. I think this just means that the wording, specifically "the error will be wrapped" needs to be changed so that it doesn't give the impression the SCE will be the cause.

:

Minor implementation suggestion for findServiceProvider is that you can use for-each to iterate through the providers. I don't mind. Do you think it's better? I'll make the changes and send an updated webrev... I think it would be neater but it's a minor comment and nothing to do with the main dish.

:

Finally one suggestion is to make keep notes on the "before & after" behavior, this is something that will be important for release and compatibility notes. OK - but in what form? In the webrev header? Or do you think I should add // comments in the code to make explicit the behavioral changes? Or do you mean I should collect & gather them in some separate text doc that will help writing the RN? Good question, I'd suggest adding a note to JDK-7169894 for now so that it is as least recorded somewhere.

-Alan

More information about the core-libs-dev mailing list