felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guillaume Nodet (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FELIX-3761) When a bundle registers a service, the bundle lock is obtained without any real purpose
Date Fri, 30 Nov 2012 08:51:58 GMT

    [ https://issues.apache.org/jira/browse/FELIX-3761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13507203#comment-13507203
] 

Guillaume Nodet commented on FELIX-3761:
----------------------------------------

I disagree.  It's not about grabbing the bundle lock inside the registry, it's just about
checking that the bundle context is still valid.
Something like:

{code}
diff --git a/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java b/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
index 4a6aa8e..357d557 100644
--- a/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
+++ b/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
@@ -118,6 +118,10 @@ public class ServiceRegistry
 
         synchronized (this)
         {
+            if (bundle.getBundleContext() == null) {
+                throw new IllegalStateException("Invalid BundleContext.");
+            }
+
             // Create the service registration.
             reg = new ServiceRegistrationImpl(
                 this, bundle, classNames, new Long(m_currentServiceId++), svcObj, dict);
{code}

It should solve all problems.  While the activator is running, the bundle will still be able
to register services from a different thread.  Once the activator has run, felix will invalidate
the bundle context, then unregister all services.  So even is a service is registered while
the activator is running (or even after it has run but before the context is invalidated),
that service will be correctly unregistered.
If the service is being registered after the bundle context has been invalidated, it will
be rejected as expected.

It may happen there is still a gap between the service registration thread being run and the
check of the bundle context validity, which means the bundle could be restarted and the service
registered.  I agree that's bad and the service registry should be given the calling bundle
context instead of the bundle itself, which would make get rid of that problem by making sure
the context is still valid.


So I think this could be solved the following way:
{code}
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
index d7bbfa0..e7a3f4e 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
@@ -343,7 +343,7 @@ class BundleContextImpl implements FelixBundleContext
             }
         }
 
-        return m_felix.registerService(m_bundle, clazzes, svcObj, dict);
+        return m_felix.registerService(this, clazzes, svcObj, dict);
     }
 
     public <S> ServiceRegistration<S> registerService(
@@ -498,7 +498,7 @@ class BundleContextImpl implements FelixBundleContext
         return m_felix.getDataFile(m_bundle, s);
     }
 
-    private void checkValidity()
+    void checkValidity()
     {
         if (m_valid)
         {
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index 08b6d90..2ad8a70 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -3435,7 +3435,7 @@ public class Felix extends BundleImpl implements Framework
      * @return A <code>ServiceRegistration</code> object or null.
     **/
     ServiceRegistration registerService(
-        BundleImpl bundle, String[] classNames, Object svcObj, Dictionary dict)
+        BundleContextImpl context, String[] classNames, Object svcObj, Dictionary dict)
     {
         if (classNames == null)
         {
@@ -3470,7 +3470,7 @@ public class Felix extends BundleImpl implements Framework
             }
         }
 
-        reg = m_registry.registerService(bundle, classNames, svcObj, dict);
+        reg = m_registry.registerService(context, classNames, svcObj, dict);
 
         // Check to see if this a listener hook; if so, then we need
         // to invoke the callback with all existing service listeners.
diff --git a/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java b/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
index 4a6aa8e..77b17b8 100644
--- a/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
+++ b/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
@@ -112,12 +112,14 @@ public class ServiceRegistry
 
     // Caller is expected to fire REGISTERED event.
     public ServiceRegistration registerService(
-        Bundle bundle, String[] classNames, Object svcObj, Dictionary dict)
+        BundleContextImpl context, String[] classNames, Object svcObj, Dictionary dict)
     {
         ServiceRegistrationImpl reg = null;
 
         synchronized (this)
         {
+            Bundle bundle = context.getBundle();
+
             // Create the service registration.
             reg = new ServiceRegistrationImpl(
                 this, bundle, classNames, new Long(m_currentServiceId++), svcObj, dict);
{code}

Thoughts ?
                
> When a bundle registers a service, the bundle lock is obtained without any real purpose
> ---------------------------------------------------------------------------------------
>
>                 Key: FELIX-3761
>                 URL: https://issues.apache.org/jira/browse/FELIX-3761
>             Project: Felix
>          Issue Type: Bug
>    Affects Versions: framework-4.0.2
>            Reporter: Guillaume Nodet
>
> It leads to the following kind of deadlocks:
> {code}
> "Felix WebConsole worker" Id=170 in WAITING on lock=[Ljava.lang.Object;@108a93d9
>     at java.lang.Object.wait(Native Method)
>     at java.lang.Object.wait(Object.java:485)
>     at org.apache.felix.framework.Felix.acquireBundleLock(Felix.java:5203)
>     at org.apache.felix.framework.Felix.registerService(Felix.java:3452)
>     at org.apache.felix.framework.BundleContextImpl.registerService(BundleContextImpl.java:346)
>     at org.apache.felix.framework.BundleContextImpl.registerService(BundleContextImpl.java:320)
>     at org.apache.felix.webconsole.internal.core.BundlesServlet.activate(BundlesServlet.java:132)
>     at org.apache.felix.webconsole.internal.servlet.PluginHolder$InternalPlugin.doGetConsolePlugin(PluginHolder.java:752)
>     at org.apache.felix.webconsole.internal.servlet.PluginHolder$Plugin.getConsolePlugin(PluginHolder.java:535)
>     at org.apache.felix.webconsole.internal.servlet.PluginHolder.getPlugin(PluginHolder.java:205)
>     at org.apache.felix.webconsole.internal.servlet.OsgiManager.initInternalPlugins(OsgiManager.java:1016)
>     at org.apache.felix.webconsole.internal.servlet.OsgiManager.doUpdateConfiguration(OsgiManager.java:981)
>     at org.apache.felix.webconsole.internal.servlet.OsgiManager$6.run(OsgiManager.java:930)
>     at org.apache.felix.webconsole.internal.servlet.Executor$Worker.run(Executor.java:67)
> "FelixFrameworkWiring" Id=168 in WAITING on lock=java.lang.Object@3d6bca49
>     at java.lang.Object.wait(Native Method)
>     at java.lang.Object.wait(Object.java:485)
>     at org.apache.felix.webconsole.internal.servlet.Executor.shutdown(Executor.java:45)
>       - locked org.apache.felix.webconsole.internal.servlet.Executor@21875b11
>     at org.apache.felix.webconsole.internal.servlet.OsgiManager.dispose(OsgiManager.java:407)
>     at org.apache.felix.webconsole.internal.KarafOsgiManagerActivator.stop(KarafOsgiManagerActivator.java:52)
>     at org.apache.felix.framework.util.SecureAction.stopActivator(SecureAction.java:667)
>     at org.apache.felix.framework.Felix.stopBundle(Felix.java:2604)
>     at org.apache.felix.framework.Felix$RefreshHelper.stop(Felix.java:4961)
>     at org.apache.felix.framework.Felix.refreshPackages(Felix.java:4203)
>     at org.apache.felix.framework.FrameworkWiringImpl.run(FrameworkWiringImpl.java:172)
>     at java.lang.Thread.run(Thread.java:680)
> {code}
> There's really no need to grab the bundle lock because the bundle is necessarily starting
/ active / stopping, else an exception would have been thrown when calling the registerService
because the BundleContext object would have been invalidated, and the registry itself is synchronized.
 

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message