Implement OSGi Service Loader Mediator Specification #295
Conversation
Signed-off-by: Hannes Wellmann <[email protected]>
|
The basic ideas could also be used to enable the usage of the |
|
I think this is a very creative way to solve the problem but I'm not sure if the resource/classlaoding is always triggered, e.g what happens if the classes are already loaded? So the best would be to add a testcase that covers all possible call variants of service loader especially those that lookup the services multiple times (also using reset())... @rotty3000 might can help on the details with the specification itself... |
| return null; | ||
| } | ||
|
|
||
| private <T> Stream<T> searchProviders(String name, ModuleClassLoader classLoader, |
There was a problem hiding this comment.
Instead of dynamically searching this list I would use BundleTracker in the Hook that collects bundles and read their META-INF/services in a datastructure that can then be used to lookup current active providers, that has several advantages:
- no need to query the wiring and iterate the bundles
- easier to debug as there is some current state to inspect
- instead of the
latestServiceNamewe can inspect if the bundle tries to load a class that is an SPI of another bundle and then inspect the stack, requiring a certain call order seems a bit fragile if something changes in the implementation of service loader.
There was a problem hiding this comment.
Instead of dynamically searching this list I would use BundleTracker in the Hook that collects bundles and read their
META-INF/servicesin a datastructure that can then be used to lookup current active providers, that has several advantages:
IIRC the service loader mediator specification requires the impl to look at the wiring to allow a bundle to "use" only the providers it is wired to.
There was a problem hiding this comment.
Instead of dynamically searching this list I would use BundleTracker in the Hook that collects bundles and read their
META-INF/servicesin a datastructure that can then be used to lookup current active providers, that has several advantages:IIRC the service loader mediator specification requires the impl to look at the wiring to allow a bundle to "use" only the providers it is wired to.
I think so as well. However having a state also allows to avoid the ThreadLocal.
In general I try to have a somewhat hybrid approach that maintains a state with all services, but when there is a query for services checks the current wires if that bundle is permitted to really obtain them.
The goal is also a bit to demonstrate how this could be done totally without (or at least with minimal) metadata as intended in osgi/osgi#372. If that issue is once resolved all the checks for wires could simply be removed and everything possible could be supplied.
| @@ -113,6 +114,7 @@ public void initialize() { | |||
| addClassLoaderHook(new DevClassLoadingHook(container.getConfiguration())); | |||
| addClassLoaderHook(new EclipseLazyStarter(container)); | |||
| addClassLoaderHook(new WeavingHookConfigurator(container)); | |||
| addClassLoaderHook(new ServiceLoaderMediatorHook()); | |||
There was a problem hiding this comment.
This should be provided as a fragment of org.eclipse.osgi like:
and use a hookconfigurators.properties to specify an implementation of org.eclipse.osgi.internal.hookregistry.HookConfigurator like
Then you can add your ClassLoaderHook with HookConfigurator.addHooks(HookRegistry)
We should keep this out of the framework as long as service loader mediator specification is not part of the OSGi Core specification.
There was a problem hiding this comment.
I considered this as well, mainly for the point you mentioned below that it would allow later Java versions than 8 (9 would already be a big win).
But the big drawback of this approach is that one has to list the fragment as framework-extension in a system-property of the runtime. For example in a launch config:
-Dosgi.framework.extensions=reference:file:${project_loc:/org.eclipse.equinox.spi}, in a product definition this would usually be -Dosgi.framework.extensions=reference:file:org.eclipse.equinox.spi, assuming the spi and o.e.osgi jar are colocated.
Or is there another way to load a framework-extension with the framework's classloader so that the hookconfigurators.properties are found?
Because actually another goal of this was to avoid the need (if possible) for any configuration that it just works out of the box.
For the build-time some p2-instructions could be added to the fragment, but PDE does not yet have any help for that as far as I know.
Tests will be needed regardless. But handling cases where the class is already loaded should not be a concern if the class is loaded by a framework class loader. This is because the hook will be in place BEFORE any class is defined from any and all bundles installed in the framework. Say something is loaded by an OSGi connect class loader? I am fairly certain there is no solution that can implement the service loader mediator in that case. Only way that will be solved generally is if we can convince Java to add something to the service loader to allow us to hook into it instead of hacking around with class loaders to do this. |
| return false; | ||
| } | ||
| return requiredWires.stream().anyMatch(w -> w.getRequirement().getDirectives().getOrDefault("filter", "") //$NON-NLS-1$//$NON-NLS-2$ | ||
| .contains("(osgi.extender=osgi.serviceloader.processor)") //$NON-NLS-1$ |
There was a problem hiding this comment.
More efficient and reliable to check the provider attribute osgi.extender equals osgi.serviceloader.processor I think and assume the filter matched it instead of trying to do a simple string compare here.
| String filterDirective = requirement.getDirectives().get("filter"); //$NON-NLS-1$ | ||
| Object serviceLoaderAttribute = wire.getCapability().getAttributes().get("osgi.serviceloader"); //$NON-NLS-1$ | ||
| return serviceName.equals(serviceLoaderAttribute) | ||
| && ("(osgi.serviceloader=" + serviceName + ")").equals(filterDirective); //$NON-NLS-1$ //$NON-NLS-2$ |
There was a problem hiding this comment.
I don't think this extra check on filter is needed.
| return null; | ||
| } | ||
|
|
||
| private <T> Stream<T> searchProviders(String name, ModuleClassLoader classLoader, |
There was a problem hiding this comment.
Instead of dynamically searching this list I would use BundleTracker in the Hook that collects bundles and read their
META-INF/servicesin a datastructure that can then be used to lookup current active providers, that has several advantages:
IIRC the service loader mediator specification requires the impl to look at the wiring to allow a bundle to "use" only the providers it is wired to.
That would be great. Do you think it would be possible to suggest such think to the OpenJDK team and describe what would be required from framework POV?
I think you are right, the only thing I can think of is if the bundle is "too late" then the user might need to reset the serviceloader to make this visible. |
That is a limitation of Equinox framework hooks, they can only be added when the framework is initialized. They cannot be added dynamically later. If using Equinox Framework Hooks then the framework must be restarted to load up the mediator implementation. |
That is supposed to be what osgi/osgi#372 was about. But there has not been much activity on that issue. |
|
I mean if an impl do severcieloaderlookup and get some classes and after that I install another bundle then there is no way to tell the consumer "here is another one" ... the hook should best be installed right away from the start of the framework. |
|
|
||
| @SuppressWarnings({ "unchecked", "rawtypes" }) | ||
| private Supplier<Optional<Class<?>>> getCallerClassComputer() { | ||
| try { // The Java-9+ way |
There was a problem hiding this comment.
If you place this in a framework fragment then we can make it depend on a higher osgi.ee than Java 8.
|
Thanks for all the feedback, I have already started to implemented some of the suggestions.
Yes, but I assume that ServiceLoader objects are not kept for a long time. I guess that the usual usage pattern is that one looks for as many instances of a service as desired and collects them and if later interested in instances again creates a fresh instance. |
|
@HannesWell any chance we can push this forward? |
Yes. I will first complete my work on the native launcher build pipeline and will then continue with this. |
In order to use bundles that use the Java
ServiceLoadermechanism an implementation of theOSGi Service Loader specificationis required at runtime. The reference implementation, which is also used by the Eclipse Platform at the moment, is Apache Aries SPI Fly, which uses class-file transformation in order to intercept calls to the ServiceLoader and needs to be configured as auto-started bundle.This PR is a simple prototype with the intend to implement the
OSGi Service Loader specificationdirectly in org.eclipse.osgi.The basic idea is to intercept
ServiceLoadercalls by using aClassLoaderHookand searching for the ServiceLoader in the caller classes if the chances are high that theClassLoader.getResource()andClass.forName()is called by the ServiceLoader.This is currently a draft, that I have implemented naively without full knowledge of the entire
OSGi Service Loader specification, so there are probably many things to work out. Nevertheless I think it already demonstrates the basic approach and at least for supplying a logging provider to theslf4j.apibundle this already works.@laeubi FYI
@tjwatson what do you think?