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 <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 <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 <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/54290/> https://git.eclipse.org/r/#/c/54257/ <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 <tjwat...@us.ibm.com> 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.blew...@gmail.com> > To: Equinox development mailing list <equinox-dev@eclipse.org> > Date: 09/25/2015 01:33 PM > Subject: [equinox-dev] OSGiServiceSupplier > Sent by: equinox-dev-boun...@eclipse.org > > > > I posted on http://alblue.bandlem.com/2015/10/osgi-services-in-java-8.html > <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@eclipse.org > To change your delivery options, retrieve your password, or unsubscribe from > this list, visit > https://dev.eclipse.org/mailman/listinfo/equinox-dev > <https://dev.eclipse.org/mailman/listinfo/equinox-dev> > > _______________________________________________ > equinox-dev mailing list > equinox-dev@eclipse.org > 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@eclipse.org To change your delivery options, retrieve your password, or unsubscribe from this list, visit https://dev.eclipse.org/mailman/listinfo/equinox-dev
