Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [equinox-dev] OSGiServiceSupplier

Alex,

What about this:

> public class OSGiServiceWrapper<S> {
> 	private BundleContext ctx;
> 	private BundleContext context;
> 	private Class<S> serviceType;
> 
> 	public OSGiServiceWrapper(Class<?> requestor, Class<S> serviceType) {
> 		this.context = FrameworkUtil.getBundle(requestor).getBundleContext();
> 		this.serviceType = serviceType;
> 	}
> 
> 	public static <S> OSGiServiceWrapper<S> create(Class<?> requestor, Class<S> serviceType) {
> 		return new OSGiServiceWrapper<>(requestor, serviceType);
> 	}
> 
> 
> 	public <V,R> Function<V,R> createFunction(BiFunction<V, S, R> f) {
> 		return (v) -> {
> 			ServiceReference<S> reference = this.context.getServiceReference(this.serviceType);
> 			if( reference != null ) {
> 				S service = this.context.getService(reference);
> 				try {
> 					return f.apply(v,service);
> 				} finally {
> 					this.context.ungetService(reference);
> 				}
> 
> 			}
> 			return (R)null;
> 		};
> 	}
> 
> 	public static void main(String[] args) {
> 		OSGiServiceWrapper<DebugOptions> options = OSGiServiceWrapper.create(OSGiServiceWrapper.class, DebugOptions.class);
> 		Function<String, Boolean> f = options.createFunction( (o,s) -> {
> 			return s.getBooleanOption(o, true);
> 		} );
> 		f.apply("Test");
> 	}
> }

Tom

On 25.09.15 23:42, Alex Blewitt wrote:
> BTW I updated the examples in my blog post to use AutoClosable and have
> a finalize method. So Tom’s comments below reflected the version before
> I made those changes.
> 
>> On 25 Sep 2015, at 22:20, Alex Blewitt <alex.blewitt@xxxxxxxxx
>> <mailto:alex.blewitt@xxxxxxxxx>> wrote:
>>
>> Thanks for the feedback. I didn’t claim that they followed best OSGi
>> practice but you make valid points about the ServiceTracker not being
>> closed. A finalize method will help with that, though will defer the
>> cleanup to a different part.
>>
>> The problem, of course, is that the java.util.function.Supplier has no
>> way of codifying a ‘close’ type resource (though perhaps this might be
>> modified if the tracker variant implements AutoClosable?).
>>
>> However, there’s a lot of cruft in Eclipse generally that are doing
>> this kind of pattern anyway. For example, the ContextsActivator
>>
>> https://github.com/eclipse/eclipse.platform.runtime/blob/master/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/osgi/ContextsActivator.java
>>
>> sets up a debug tracker and starts/stops it in the Activator (which
>> I’m trying to remove). Yet the code only gets used in DebugHelper:
>>
>> https://github.com/eclipse/eclipse.platform.runtime/blob/master/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/DebugHelper.java
>>
>> and even then it only calls it in a static block, once.
>>
>> So what’s happened here is:
>>
>> * Someone read up on ‘best practices’
>> * They set up a tracker
>> * They initialised it in the start and stop in the activator, as best
>> practice suggests
>> * They used it in an irrelevant way because that was the easiest way
>> to get the service
>>
>> I argue that in this specific case, it’s better to perform a one-off
>> lookup of the service instead of keeping a tracker for evermore:
>>
>> https://git.eclipse.org/r/#/c/56739/1/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/DebugHelper.java
>>
>> (Subsequent patch removes the class completely because it’s never used …)
>>
>> I argue that acquiring a service with a Supplier moves the
>> implementation from how to pick up the service into the implementation
>> instead of the class. In this case, the single-shot approach can be used.
>>
>> On the other hand, something like FrameworkLog is probably something
>> to keep around for a while instead of looking up each time.
>>
>> https://git.eclipse.org/r/#/c/56582/2/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/osgi/LogHelper.java
>>
>> These are in static references already (mainly to avoid the problems
>> with looking up services the whole time). These methods are repeated
>> time and time again throughout the Eclipse codebase. Yes, you can
>> argue it’s not best practice, and yes you can argue that DS is
>> probably better in most cases. But I can only delete code so quickly :)
>>
>> On the plus side, I am making progress in such situations - for
>> example, I decoupled static references from contenttype and moved from
>> bundle-activator lookups to references passed in from declarative
>> services as well.
>>
>> https://git.eclipse.org/r/#/c/54290/ 
>> https://git.eclipse.org/r/#/c/54257/
>>
>> I agree that the get/unget combo will cause oscillations; but the
>> options are between going from zero to one and to zero again, or going
>> from zero to one and leaking that way. 
>>
>> In summary;
>>
>> * Yes, they could be improved
>> * No, they don’t exhibit best OSGi practices
>> * This code is being used in Eclipse platform whether or not there is
>> a standard implementation for the above or not
>> * They are intended to replace patterns where existing services are
>> kept for the lifetime of the bundle anyway
>> * For one-shot services (such as debugoptions usecases) a
>> servicetracker is probably overkill anyway
>> * For ongoing or frequent uses, the servicetracker option will
>> probably be the best. Such references are (commonly) stored in static
>> variables at the moment, which will be coupled with the lifetime of
>> the class
>>
>> I wonder how much of the above could be mitigated with the appropriate
>> documentation covering when and how they should be used.
>>
>> Alex
>>
>>> On 25 Sep 2015, at 21:45, Thomas Watson <tjwatson@xxxxxxxxxx
>>> <mailto:tjwatson@xxxxxxxxxx>> wrote:
>>>
>>> I have some issues with both examples you gave.  For the code that
>>> uses ServiceTracker it will end up creating a ServiceTracker and
>>> never closing it which will leave a ServiceListener leaked
>>> (registered) with the framework for the lifetime of the active
>>> bundle.  Once the bundle is stopped the leaked listeners will finally
>>> get cleared.  So if you use the OSGiTracker in a short lived object
>>> that gets created often then you will quickly grind the service event
>>> bus of the framework to a halt.  You may try to limit the damage of
>>> that leak by having a finalize() method that closes the tracker, but
>>> usually finalize() methods are not recommended best-practice.
>>>
>>> The OSGiSupplier is better but it has the unfortunate side-effect of
>>> ungetting the service object before even returning it to the client
>>> code for its first usage.  In general this is not using good
>>> practices in OSGi because the service registration may be using a
>>> ServiceFactory that needs to track the state of the using bundles.
>>>  Such usage on a ServiceFactory registration will cause the service
>>> usecount for the bundle to osilate between one and zero within the
>>> get() method.  Each time the usecount goes to zero the service object
>>> is thrown away by the framework, then on next usage the service
>>> object will have to be recreated by the factory.  This could result
>>> in a performance issue if the creating of the service object is
>>> expensive.
>>>
>>> The classes may have their uses in specific cases and not cause
>>> issues if used in a specific way.  For example, if you know for a
>>> fact that the service you are getting is not stateful and does not
>>> use a ServiceFactory.  Or if you make sure to use the OSGiTracker in
>>> an object that has a one-to-one relationship with the active
>>> lifecycle of the bundle.  I am not sure they belong down in Equinox
>>> though because I do not believe they promote best-practices when
>>> dealing with the OSGi service registry.
>>>
>>> I also wonder if the latest DS specification helps deal with some of
>>> the short-comings you mention in your blog.
>>>
>>> Finally I do thank you for proposing a solution to a problem and
>>> bringing it here for discussion, I just don't feel comfortable with
>>> the current solution :-)
>>>
>>> Tom
>>>
>>>
>>>
>>>
>>>
>>> From:        Alex Blewitt <alex.blewitt@xxxxxxxxx
>>> <mailto:alex.blewitt@xxxxxxxxx>>
>>> To:        Equinox development mailing list <equinox-dev@xxxxxxxxxxx
>>> <mailto:equinox-dev@xxxxxxxxxxx>>
>>> Date:        09/25/2015 01:33 PM
>>> Subject:        [equinox-dev] OSGiServiceSupplier
>>> Sent by:        equinox-dev-bounces@xxxxxxxxxxx
>>> <mailto:equinox-dev-bounces@xxxxxxxxxxx>
>>> ------------------------------------------------------------------------
>>>
>>>
>>>
>>> I posted on
>>> _http://alblue.bandlem.com/2015/10/osgi-services-in-java-8.html_earlier
>>> today about using Java 8’s Supplier to acquire a service on-demand
>>> without having to do expensive client-side coding, and that would
>>> fail fast in the absence of any OSGi classes and return null in such
>>> situations.
>>>
>>> I’d like to contribute this to Eclipse so that it can be used in
>>> places where Declarative Services aren’t the right solution
>>> (specifically; for integrating in where places have static Log or
>>> DebugOptions classes).
>>>
>>> Which would be the right project/bundle to contribute this into?
>>> Clearly it could go into something like Platform or Core.runtime, but
>>> I wondered if it might be sensible to have this in equinox.util or
>>> equivalent.
>>>
>>> Alex_______________________________________________
>>> equinox-dev mailing list
>>> equinox-dev@xxxxxxxxxxx <mailto:equinox-dev@xxxxxxxxxxx>
>>> To change your delivery options, retrieve your password, or
>>> unsubscribe from this list, visit
>>> https://dev.eclipse.org/mailman/listinfo/equinox-dev
>>>
>>> _______________________________________________
>>> equinox-dev mailing list
>>> equinox-dev@xxxxxxxxxxx <mailto:equinox-dev@xxxxxxxxxxx>
>>> To change your delivery options, retrieve your password, or
>>> unsubscribe from this list, visit
>>> https://dev.eclipse.org/mailman/listinfo/equinox-dev
>>
> 
> 
> 
> _______________________________________________
> equinox-dev mailing list
> equinox-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/equinox-dev
> 


-- 
Thomas Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
http://www.bestsolution.at/
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck


Back to the top