felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Jencks <david_jen...@yahoo.com.INVALID>
Subject Re: svn commit: r1684594 - /felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
Date Wed, 10 Jun 2015 22:10:59 GMT
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 <david.bosschaert@gmail.com> wrote:

> Hi David,
> 
> On 10 June 2015 at 17:42, David Jencks <david_jencks@yahoo.com.invalid> 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


Mime
View raw message