Re: svn commit: r1509692 - /felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java

["Richard S. Hall" <he...@ungoverned.org>]

On 8/2/13 09:23 , gnodet@apache.org wrote:
> Author: gnodet
> Date: Fri Aug  2 13:23:13 2013
> New Revision: 1509692
>
> URL: http://svn.apache.org/r1509692
> Log:
> [FELIX-4190] The framework should not hold any lock while calling ServiceFactory#unget
>
> This commit should be quite safe as getUsingBundles() and ungetService() are already thread safe, and there's no possibility for the bundle to acquire a new reference to the service because the service has already been unregistered

Technically, I think this patch is not correct.

We specifically allow for bundles that already hold the service 
reference to acquire the service object while a service is being 
unregistered (see comments in ServiceRegistry.unregisterService()). This 
means that a bundle that already holds the service reference could, 
technically, acquire the unregistering service just after you call 
getUsingBundles(), thus we wouldn't be forcing it to unget.

The only way I think we can do what you want is to somehow acquire the 
registry lock (i.e., this) again and put us into some new state that 
doesn't allow any more bundles to acquire the unregistering service...or 
perhaps by adding something to the service registration to put it into a 
new state where it won't return anything from getService. Then we could 
release the registry lock and continue with what you have.

-> richard

>
> Modified:
>      felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
>
> Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
> URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java?rev=1509692&r1=1509691&r2=1509692&view=diff
> ==============================================================================
> --- felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java (original)
> +++ felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java Fri Aug  2 13:23:13 2013
> @@ -153,16 +153,13 @@ public class ServiceRegistry
>           }
>   
>           // Now forcibly unget the service object for all stubborn clients.
> -        synchronized (this)
> +        Bundle[] clients = getUsingBundles(reg.getReference());
> +        for (int i = 0; (clients != null) && (i < clients.length); i++)
>           {
> -            Bundle[] clients = getUsingBundles(reg.getReference());
> -            for (int i = 0; (clients != null) && (i < clients.length); i++)
> -            {
> -                while (ungetService(clients[i], reg.getReference()))
> -                    ; // Keep removing until it is no longer possible
> -            }
> -            ((ServiceRegistrationImpl) reg).invalidate();
> +            while (ungetService(clients[i], reg.getReference()))
> +                ; // Keep removing until it is no longer possible
>           }
> +        ((ServiceRegistrationImpl) reg).invalidate();
>       }
>   
>       /**
>
>