Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2020-05-15 14:02:05 +0000
committerThomas Watson2020-05-15 21:10:49 +0000
commit01ffcf50611440654461f402bb1dd3008477f531 (patch)
tree708ef6fd3f3e6173720b378b0d5906927966d9a9
parent638d4556eb9f14b36f3aff06cb7566d48e3ec1eb (diff)
downloadrt.equinox.framework-01ffcf50611440654461f402bb1dd3008477f531.tar.gz
rt.equinox.framework-01ffcf50611440654461f402bb1dd3008477f531.tar.xz
rt.equinox.framework-01ffcf50611440654461f402bb1dd3008477f531.zip
Bug 553773 - Avoid calling hooks while registeringI20200516-0600I20200515-1930
This avoids getting the framework hook until after it has finished registering. Some hooks run into issues if they are allowed to be called while they are registering. For example, WeavingHooks that are registered by declarative services can cause issues with re-entering into activation by SCR. This change also avoids the constant get/unget service hooks registrations each time the hook is invoked. Hooks are high traffic services during event storms. This should improve performance when hooks are present. Change-Id: Ic937616d421f1d5944b614a91e74417070f06fd0 Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java4
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java55
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java50
3 files changed, 100 insertions, 9 deletions
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
index 2f509b25d..f279c8c3c 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2006, 2018 IBM Corporation and others.
+ * Copyright (c) 2006, 2020 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -2283,7 +2283,7 @@ public class ClassLoadingBundleTests extends AbstractBundleTests {
reg.unregister();
}
- assertEquals("Unexpected number of woven classes.", 3, called.size());
+ assertEquals("Unexpected number of woven classes.", 2, called.size());
for (WovenClass wovenClass : called) {
if (weavingHookClasses.contains(wovenClass.getClassName())) {
assertNull("Did not expect to find class: " + wovenClass.getDefinedClass(), wovenClass.getDefinedClass());
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
index c81a64bfd..0d1e51e2a 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
@@ -234,6 +234,7 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
}
}
+ ungetHookInstance();
/* must not hold the registrationLock when this event is published */
registry.publishServiceEvent(new ServiceEvent(ServiceEvent.UNREGISTERING, ref));
@@ -291,6 +292,18 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
return getReferenceImpl();
}
+ S getHookInstance() {
+ return null;
+ }
+
+ void initHookInstance() {
+ // nothing by default
+ }
+
+ void ungetHookInstance() {
+ // nothing by default
+ }
+
ServiceReferenceImpl<S> getReferenceImpl() {
/* use reference instead of unregistered so that ServiceFactorys, called
* by releaseService after the registration is unregistered, can
@@ -759,4 +772,46 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
}
return 1;
}
+
+ static class FrameworkHookRegistration<S> extends ServiceRegistrationImpl<S> {
+ private volatile boolean hookInitialized = false;
+ private volatile S hookInstance;
+ private final BundleContextImpl systemContext;
+ private final Object hookLock = new Object();
+
+ FrameworkHookRegistration(ServiceRegistry registry, BundleContextImpl context, String[] clazzes, S service,
+ BundleContextImpl systemContext) {
+ super(registry, context, clazzes, service);
+ this.systemContext = systemContext;
+ }
+
+ @Override
+ S getHookInstance() {
+ if (hookInstance != null || !hookInitialized) {
+ return hookInstance;
+ }
+ synchronized (hookLock) {
+ if (hookInstance == null) {
+ hookInstance = getSafeService(systemContext, ServiceConsumer.singletonConsumer);
+ }
+ }
+ return hookInstance;
+ }
+
+ @Override
+ void initHookInstance() {
+ ServiceReference<S> ref = getReference();
+ if (ref != null) {
+ hookInstance = getSafeService(systemContext, ServiceConsumer.singletonConsumer);
+ hookInitialized = true;
+ }
+ }
+
+ @Override
+ void ungetHookInstance() {
+ if (hookInstance != null) {
+ systemContext.ungetService(getReferenceImpl());
+ }
+ }
+ }
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
index 36ceb9790..934cc0cc6 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2004, 2017 IBM Corporation and others.
+ * Copyright (c) 2004, 2020 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -74,6 +74,7 @@ public class ServiceRegistry {
static final String eventListenerHookName = EventListenerHook.class.getName();
static final String listenerHookName = ListenerHook.class.getName();
+
/** Published services by class name.
* The {@literal List<ServiceRegistrationImpl<?>>}s are both sorted
* in the natural order of ServiceRegistrationImpl and also are sets in that
@@ -229,12 +230,16 @@ public class ServiceRegistry {
throw new IllegalArgumentException(Msg.SERVICE_EMPTY_CLASS_LIST_EXCEPTION);
}
+ boolean isListenerHook = false;
+ boolean isFrameworkHook = false;
/* copy the array so that changes to the original will not affect us. */
List<String> copy = new ArrayList<>(size);
// intern the strings and remove duplicates
for (int i = 0; i < size; i++) {
String clazz = clazzes[i].intern();
if (!copy.contains(clazz)) {
+ isListenerHook = isListenerHook || listenerHookName.equals(clazz);
+ isFrameworkHook = isFrameworkHook || isFrameworkHook(clazz);
copy.add(clazz);
}
}
@@ -254,14 +259,44 @@ public class ServiceRegistry {
}
}
- ServiceRegistrationImpl<?> registration = new ServiceRegistrationImpl<>(this, context, clazzes, service);
+ ServiceRegistrationImpl<?> registration = isFrameworkHook
+ ? new ServiceRegistrationImpl.FrameworkHookRegistration<>(this, context, clazzes, service,
+ systemBundleContext)
+ : new ServiceRegistrationImpl<>(this, context, clazzes, service);
registration.register(properties);
- if (copy.contains(listenerHookName)) {
+ registration.initHookInstance();
+
+ if (isListenerHook) {
notifyNewListenerHook(registration);
}
return registration;
}
+ private boolean isFrameworkHook(String className) {
+ switch (className) {
+ case "org.osgi.framework.hooks.bundle.CollisionHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.bundle.EventHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.bundle.FindHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.service.EventHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.service.EventListenerHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.service.FindHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.service.ListenerHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.weaving.WeavingHook": //$NON-NLS-1$
+ return true;
+ case "org.osgi.framework.hooks.weaving.WovenClassListener": //$NON-NLS-1$
+ return true;
+ default:
+ return false;
+ }
+ }
+
/**
* Returns an array of <code>ServiceReferenceImpl</code> objects. The returned
* array of <code>ServiceReferenceImpl</code> objects contains services that
@@ -1335,8 +1370,11 @@ public class ServiceRegistry {
if (hookContext.skipRegistration(registration)) {
return;
}
- Object hook = registration.getSafeService(context, ServiceConsumer.singletonConsumer);
- if (hook == null) { // if the hook is null
+ Object hook = registration.getHookInstance();
+ if (hook == null) {
+ // The hook may not be initialized yet
+ // We do not call the hook until after it has been registered
+ // This means we could miss calls to a hook during the registered event.
return;
}
try {
@@ -1350,8 +1388,6 @@ public class ServiceRegistry {
container.handleRuntimeError(t);
ServiceException se = new ServiceException(NLS.bind(Msg.SERVICE_FACTORY_EXCEPTION, hook.getClass().getName(), hookContext.getHookMethodName()), t);
container.getEventPublisher().publishFrameworkEvent(FrameworkEvent.ERROR, registration.getBundle(), se);
- } finally {
- registration.ungetService(context, ServiceConsumer.singletonConsumer, null);
}
}

Back to the top