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 <[email protected]> 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 > > <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 > > <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 <[email protected]> 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 <[email protected]> >> To: Equinox development mailing list <[email protected]> >> Date: 09/25/2015 01:33 PM >> Subject: [equinox-dev] OSGiServiceSupplier >> Sent by: [email protected] >> >> >> >> 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 >> [email protected] >> 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 >> [email protected] >> 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 [email protected] To change your delivery options, retrieve your password, or unsubscribe from this list, visit https://dev.eclipse.org/mailman/listinfo/equinox-dev
