ServiceLoader.load(Class, ClassLoader) bug

[Paul Sandoz]

Hi Peter,

On Oct 1, 2012, at 7:00 PM, Peter Levart <peter.levart at gmail.com> wrote:

> Hi Paul,
> 
> It would be very strange code indeed that acted on the ServiceConfigurationError in a way that you describe. After all, the java.lang.Error hierarchy of throwables is reserved for things that should be "acted upon" only by fixing the code, isn't it?
> 
> This is also explicitly noted in the javadoc of the ServiceLoader:
> 
> "Design Note: Throwing an error in these cases may seem extreme. The rationale for this behaviour is that a malformed provider-configuration file, like a malformed class file, indicates a serious problem with the way the Java virtual machine is configured or is being used. As such it is preferable to throw an error rather than try to recover or, even worse, fail silently."
> 

In some cases one wants to keep on iterating when an SCE is thrown for hasNext/next e.g. perhaps log the error and move on.

In other cases if no service instances are found (in the face of errors or otherwise) there might be a fallback plan to some default behaviour.

Or when trying to convert code to use ServiceLoader that previously used some proprietary mechanism compatible META-INF/services fiies (e.g. like modifying JAXP or JAX-WS to use SL) while retaining the same behaviour.

So it's not always quite so clear cut.


> FWIW, I stumbled on the bug by chance in an application using Jetty servlet engine as an embedded component. The service that was to be loaded was a component of the Spring web application framework which passed  the "Web Context" CL to the ServiceLoader to load a component and that "Web Context" CL returned by embedded Jetty was null (probably because of miss-configuration of Jetty).
> 

Thanks, that's very useful to know.


> If ServiceLoader used System CL to load the class instead, the miss-configuration of Jetty would go unnoticed.
> 
> But that's not the situation that you are concerned about. You are concerned that a (badly written app) might end up loading "different" service implementation even though that might be the one that should be loaded (by the book).
> 

Badly written, weirdly written.

The JavaDoc for Thread. getContextClassLoader() states:

Returns:
the context ClassLoader for this Thread, or null indicating the system class loader (or, failing that, the bootstrap class loader)

Given the JavDoc on TCCL, what you say about Jetty and that JDK 8 is a major release I am inclined to think we can make such a change. I have convinced myself at least :-) i will send a webrev out.

Paul.