[
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
