On 6/25/2012 9:34 AM, Paul Sandoz wrote:
H Joe,

I just focused on javax.xml.datatype and assumed the rest follows the same 
pattern :-)

Yeah, they are mostly similar, except Schema and XPath that do a little extra check.


Like Alan i am still not sure about swallowing the ServiceConfigurationError. 
It enables something that did not work before so i guess it does not take 
anything away, if choose that this should be part of the spec then we should 
document it (as we should the dependence on TCCL).

Agree. It would be a compatibility concern.


Also i notice that the previous code that explicitly loaded META-INF/service 
files deferred to SecuritySupport that wraps stuff in 
AccessController.doPrivileged blocks. Do we need to wrap the use of 
ServiceLoader in such blocks too? since ServiceLoader will internally call 
ClassLoader.getResources() and Thread.currentThread().getContextClassLoader().

Good catch, yes we do! We're virtually delegating those calls to the ServiceLoader that could be blocked when security is on.

Joe


Paul.

On Jun 25, 2012, at 9:38 AM, Joe Wang wrote:

Hi all,

Thanks for all the comments!

I've updated the patch for the following recommended changes:
1. ServiceConfigurationError instead of ConfigurationError
2. Use factory class, class.forName section removed
3. Use load method without specifying classloader
4. To be clear on how the process handles exceptions, I've updated JavaDoc with 
the following:

     * Uses the service-provider loading facilities, defined by the {@link 
java.util.ServiceLoader} class, to attempt
     * to locate and load an implementation of the service. In case multiple 
providers exist, the first
     * valid 3rd party provider should be returned; If there is no valid 3rd 
party provider,
     * the default implementation is returned if it is on the classpath or 
installed as a module.
     *
     * If ServiceConfigurationError is thrown, it is interpreted as an invalid 
provider is encountered.
     * The process shall continue the search until all are exhausted.

     The JavaDoc is similar but slightly different for SchemaFactory and 
XPathFactory:

     *<p>Uses the service-provider loading facilities, defined by the {@link 
java.util.ServiceLoader} class, to attempt
     * to locate and load an implementation of the service.
     *     Each potential service provider is required to implement the 
method:</p>
     *<pre>
     *        {@link #isSchemaLanguageSupported(String schemaLanguage)}
     *</pre>
     *     A service provider is deemed as valid if it supports the specified 
schema language.
     *
     *<p>
     * In case multiple providers exist, the first
     * valid 3rd party provider should be returned; If there is no valid 3rd 
party provider,
     * the default implementation is returned if it is on the classpath or 
installed as a module.
     *</p>
     * If ServiceConfigurationError is thrown, it is interpreted as an invalid 
provider is encountered.
     * The process shall continue the search until all are exhausted.

We can continue discussing on a solution for the duplication of the classes. 
For this patch, if you agree, I'd like to check it in as it stands now and work 
out another patch for the duplicated classes once we have a solution.

New webrev:
http://cr.openjdk.java.net/~joehw/jdk8/7169894/webrev/

Thanks,
Joe


On 6/22/2012 10:24 AM, Joe Wang wrote:

On 6/22/2012 2:39 AM, Alan Bateman wrote:
On 21/06/2012 22:53, Joe Wang wrote:
:

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 :)  ).
No, this isn't for jdk7 or older.

I don't fully grok the issue in 6975142. That looks to me to be a case where 
the service provider implementation is including its own copy of the 
javax.xml.stream.* classes and this is the cause of the ClassCastException.
Yes, it's GF packaged woodstox that included its own API packages.

Joe

-Alan.

Reply via email to