Issue #ServiceLoaderEnhancement

[Stephen Colebourne]

On 13 July 2016 at 19:06,  <mark.reinhold at oracle.com> wrote:
> 2016/7/1 2:27:39 -0700, Stephen Colebourne <scolebourne at joda.org>:
>> Firstly, I propose that all reflection by ServiceLoader should use
>> setAccessible(true) to allow providers to be non-public.
>
> We could do this for service providers defined in modules, but not for
> those specified via the META-INF/services convention.  The problem with
> the latter is that if ServiceLoader could instantiate an arbitrary
> non-public class then an adversary could spoof it into doing that simply
> by placing an appropriate META-INF/services configuration file on the
> class path.  That's not possible for providers in modules, since the
> declaration of the provider and the provider itself are always tightly
> associated with each other, in the same artifact, so we can relax the
> restriction in that case.

I'm happy with this restriction if I understand it correctly, ie. that
access to non-public is allowed only if the provider is specified
using the module-info "provides" section, and not via
META-INF/services.

>> Secondly, I propose that if no public no-args constructor is found on
>> the provider class, ServiceLoader should locate a public static
>> constant named "INSTANCE" and use that instead.
>
> That's reasonable, though we don't have much precedence for name-based
> conventions in the SE platform itself.
> ...
> An alternative is to take an explicitly-declarative approach, defining
> something like a @Provides annotation to identify the constructors,
> factories, or fields that construct or contain actual provider objects.
> I'll give this some thought.

If it is an annotation, I would not expect to also declare it in
module-info, as that would be redundant. But as a solution it would be
fairly neat.

There is a more explicit alternative to the module-info defined approach:

module org.threeten.extra {
    requires java.base;
    exports org.threeten.extra.*;
    provides java.time.chrono.Chronology with
org.threeten.extra.chrono.CopticChronology::INSTANCE
}

The format would be ::staticConstant or ::staticMethod, using a syntax
along the lines of Java 8 method references. This would precisely
specify the desired constant/method without needing to add a
name-based convention or search. It would also allow a class to have
multiple methods/constants defining the same service. This would work
with META-INF/services as well AFAICT.

>> Thirdly, it should be possible to access the set of provider classes
>> without instantiating them. This would allow applications to use the
>> module-info design for services without using ServiceLoader, eg to
>> reflect on the provider classes, perhaps to search for annotations.
>
> Most of the services defined and used in the JDK itself follow this
> advice, but I accept that many others out in the world do not, so at
> this point people reasonably want to be able to filter providers based
> on types and annotations thereon.  This should be straightforward; I'll
> work up a proposal.

Great, thanks

Stephen