Hi, I would extremely strongly prefer that we always use jira issues for code changes. For changes in really tricky places like this code over which many people have tripped I think it's essential.
I think the comment made more sense in an earlier version of the code that used locking. With the current code, both before and after your change, it's not clear to me how we make sure a concurrent getService() from the same bundle avoids returning the object we just returned to the service factory. Although I don't really understand the code, your change at least looks very plausible. thanks david jencks On Jun 10, 2015, at 3:25 PM, David Bosschaert <[email protected]> wrote: > Hi David, > > On 10 June 2015 at 17:42, David Jencks <[email protected]> wrote: >> What is the jira number for this change? > > There is no JIRA number for this change and I'm not sure it's > absolutely required to have a JIRA number for changes. > If you want to associate a JIRA with it, it would be FELIX-4866. I'm > currently hunting down some service registry issues that only happen > in certain cases in very large test suites. > >> From the patch, your appear to have removed the logic implementing >>> - // Decrement usage count, which spec says should happen >>> after >>> - // ungetting the service object. >> >> Did you put this logic somewhere else not that I don't see from the patch? >> If not, why is this acceptable? > > Well I think that comment is a little misplaced there. The counter > that is decremented there is only used as an internal implementation > detail and doesn't really surface on the outside (unless I'm totally > mistaken), so I don't think the spec has anything to say about it. > > Cheers, > > David
