|Re: [equinox-dev] OSGiServiceSupplier|
|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
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:
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:
(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.
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.
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.
* 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.