This is an automated email from the ASF dual-hosted git repository. amichair pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/aries-rsa.git
commit d874b29c91befa3769be6557494cbc497e36ffa9 Author: Amichai Rothman <[email protected]> AuthorDate: Wed Mar 25 16:08:00 2026 +0200 ARIES-2208 Fix incorrect closing of RSA factory/core/instance/registrations --- .../rsa/core/DistributionProviderTracker.java | 42 +++--- .../aries/rsa/core/ExportRegistrationImpl.java | 20 ++- .../aries/rsa/core/ImportRegistrationImpl.java | 23 +++- .../aries/rsa/core/RemoteServiceAdminCore.java | 141 ++++----------------- .../aries/rsa/core/RemoteServiceAdminFactory.java | 27 +++- .../aries/rsa/core/RemoteServiceAdminInstance.java | 74 +++++++---- .../rsa/core/DistributionProviderTrackerTest.java | 8 +- 7 files changed, 160 insertions(+), 175 deletions(-) diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/DistributionProviderTracker.java b/rsa/src/main/java/org/apache/aries/rsa/core/DistributionProviderTracker.java index 12505e02..72714774 100644 --- a/rsa/src/main/java/org/apache/aries/rsa/core/DistributionProviderTracker.java +++ b/rsa/src/main/java/org/apache/aries/rsa/core/DistributionProviderTracker.java @@ -21,6 +21,7 @@ package org.apache.aries.rsa.core; import static org.osgi.service.remoteserviceadmin.RemoteConstants.REMOTE_CONFIGS_SUPPORTED; import static org.osgi.service.remoteserviceadmin.RemoteConstants.REMOTE_INTENTS_SUPPORTED; +import java.util.Arrays; import java.util.Dictionary; import java.util.Hashtable; @@ -37,8 +38,17 @@ import org.osgi.util.tracker.ServiceTracker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@SuppressWarnings("rawtypes") -public class DistributionProviderTracker extends ServiceTracker<DistributionProvider, ServiceRegistration> { +/** + * Tracks registered DistributionProviders, and for each one registers + * a {@link RemoteServiceAdminFactory} with an associated {@link RemoteServiceAdminCore} + * so that each TopologyManager will be given its own {@link RemoteServiceAdminInstance} + * to manage its exports/imports (backed by the shared core). + * <p> + * When the DistributionProvider is unregistered, we unregister its factory, + * which causes the framework to invoke ungetService for all of its instances, + * so they all shut down cleanly, close their exports/imports, etc. + */ +public class DistributionProviderTracker extends ServiceTracker<DistributionProvider, ServiceRegistration<RemoteServiceAdminFactory>> { private static final Logger LOG = LoggerFactory.getLogger(DistributionProviderTracker.class); public DistributionProviderTracker(BundleContext context) { @@ -46,25 +56,26 @@ public class DistributionProviderTracker extends ServiceTracker<DistributionProv } @Override - public ServiceRegistration addingService(ServiceReference<DistributionProvider> reference) { + @SuppressWarnings("unchecked") + public ServiceRegistration<RemoteServiceAdminFactory> addingService(ServiceReference<DistributionProvider> reference) { DistributionProvider provider = context.getService(reference); if (provider == null) { // Can happen if the service is created by a service factory and an exception occurs return null; } - LOG.debug("RemoteServiceAdmin Implementation is starting up"); + LOG.debug("Initializing RemoteServiceAdmin for DistributionProvider {} ({})", + provider, Arrays.asList(provider.getSupportedTypes())); BundleContext apiContext = getAPIContext(); EventProducer eventProducer = new EventProducer(context); - RemoteServiceAdminCore rsaCore = new RemoteServiceAdminCore(context, - apiContext, - eventProducer, - provider); + // we create one RSA core per tracked provider, accessed by its corresponding ServiceFactory + RemoteServiceAdminCore rsaCore = new RemoteServiceAdminCore(context, apiContext, eventProducer, provider); RemoteServiceAdminFactory rsaf = new RemoteServiceAdminFactory(rsaCore); Dictionary<String, Object> props = new Hashtable<>(); props.put(REMOTE_INTENTS_SUPPORTED, getPropertyNullSafe(reference, REMOTE_INTENTS_SUPPORTED)); props.put(REMOTE_CONFIGS_SUPPORTED, getPropertyNullSafe(reference, REMOTE_CONFIGS_SUPPORTED)); - LOG.info("Registering RemoteServiceAdmin for provider {}", provider.getClass().getName()); - return context.registerService(RemoteServiceAdmin.class.getName(), rsaf, props); + LOG.info("Registering RemoteServiceAdmin factory for provider {}", provider.getClass().getName()); + return (ServiceRegistration<RemoteServiceAdminFactory>) + context.registerService(RemoteServiceAdmin.class.getName(), rsaf, props); } private Object getPropertyNullSafe(ServiceReference<DistributionProvider> reference, String key) { @@ -84,10 +95,11 @@ public class DistributionProviderTracker extends ServiceTracker<DistributionProv @Override public void removedService(ServiceReference<DistributionProvider> reference, - ServiceRegistration reg) { - LOG.debug("RemoteServiceAdmin Implementation is shutting down now"); - reg.unregister(); - super.removedService(reference, reg); + ServiceRegistration<RemoteServiceAdminFactory> factoryRegistration) { + LOG.debug("Unregistering RemoteServiceAdmin factory for removed DistributionProvider"); + // the provider is gone - unregister its corresponding factory, + // which will also cause all of its RSA instances (and their imports/exports) to be closed + factoryRegistration.unregister(); + super.removedService(reference, factoryRegistration); } - } diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java b/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java index a2fd7129..3c919c20 100644 --- a/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java +++ b/rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java @@ -21,9 +21,11 @@ package org.apache.aries.rsa.core; import java.io.Closeable; import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.aries.rsa.core.event.EventProducer; @@ -48,13 +50,19 @@ public class ExportRegistrationImpl implements ExportRegistration { */ private static class Shared { - private CloseHandler closeHandler; + private Set<CloseHandler> closeHandlers = Collections.newSetFromMap(new ConcurrentHashMap<>()); private EventProducer eventProducer; private Closeable server; private Throwable exception; private int instanceCount; + void addCloseHandler(CloseHandler closeHandler) { + if (closeHandler != null) { + closeHandlers.add(closeHandler); + } + } + void addInstance() { synchronized (this) { instanceCount++; @@ -117,9 +125,9 @@ public class ExportRegistrationImpl implements ExportRegistration { CloseHandler closeHandler, EventProducer eventProducer) { exportReference = new ExportReferenceImpl(sref, endpoint.description()); shared = new Shared(); - shared.closeHandler = closeHandler; shared.eventProducer = eventProducer; shared.server = endpoint; + addCloseHandler(closeHandler); shared.addInstance(); } @@ -135,9 +143,9 @@ public class ExportRegistrationImpl implements ExportRegistration { */ public ExportRegistrationImpl(Throwable exception, CloseHandler closeHandler, EventProducer eventProducer) { shared = new Shared(); - shared.closeHandler = closeHandler; shared.eventProducer = eventProducer; shared.exception = exception; + addCloseHandler(closeHandler); shared.addInstance(); } @@ -167,6 +175,10 @@ public class ExportRegistrationImpl implements ExportRegistration { return closed ? null : shared.exception; } + public void addCloseHandler(CloseHandler closeHandler) { + shared.addCloseHandler(closeHandler); + } + @Override public final void close() { // we do this in two steps: first the 'closing' state to make sure we @@ -178,7 +190,7 @@ public class ExportRegistrationImpl implements ExportRegistration { if (closed || closing.getAndSet(true)) { return; } - shared.closeHandler.onClose(this); + shared.closeHandlers.forEach(h -> h.onClose(this)); if (exportReference != null) { exportReference.close(); } diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/ImportRegistrationImpl.java b/rsa/src/main/java/org/apache/aries/rsa/core/ImportRegistrationImpl.java index ef5e4189..589e37f0 100644 --- a/rsa/src/main/java/org/apache/aries/rsa/core/ImportRegistrationImpl.java +++ b/rsa/src/main/java/org/apache/aries/rsa/core/ImportRegistrationImpl.java @@ -18,9 +18,8 @@ */ package org.apache.aries.rsa.core; -import java.util.ArrayList; -import java.util.Hashtable; -import java.util.List; +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.aries.rsa.core.event.EventProducer; @@ -50,15 +49,21 @@ public class ImportRegistrationImpl implements ImportRegistration, ImportReferen */ private static class Shared { - private volatile EndpointDescription endpoint; - private CloseHandler closeHandler; + private Set<CloseHandler> closeHandlers = Collections.newSetFromMap(new ConcurrentHashMap<>()); private EventProducer eventProducer; + private volatile EndpointDescription endpoint; private volatile Throwable exception; private volatile ClientServiceFactory clientServiceFactory; private volatile ServiceRegistration importedService; // all linked import registrations that share this state private final List<ImportRegistrationImpl> instances = new ArrayList<>(1); + void addCloseHandler(CloseHandler closeHandler) { + if (closeHandler != null) { + closeHandlers.add(closeHandler); + } + } + /** * Add a linked ImportRegistration instance to the shared data. * @@ -129,8 +134,8 @@ public class ImportRegistrationImpl implements ImportRegistration, ImportReferen public ImportRegistrationImpl(EndpointDescription endpoint, CloseHandler closeHandler, EventProducer eventProducer) { shared = new Shared(); shared.endpoint = endpoint; - shared.closeHandler = closeHandler; shared.eventProducer = eventProducer; + addCloseHandler(closeHandler); shared.addInstance(this); } @@ -189,6 +194,10 @@ public class ImportRegistrationImpl implements ImportRegistration, ImportReferen return shared.exception != null || closed; } + public void addCloseHandler(CloseHandler closeHandler) { + shared.addCloseHandler(closeHandler); + } + /** * Closes this instance. * <p> @@ -207,7 +216,7 @@ public class ImportRegistrationImpl implements ImportRegistration, ImportReferen if (closed || closing.getAndSet(true)) { return; } - shared.closeHandler.onClose(this); + shared.closeHandlers.forEach(h -> h.onClose(this)); shared.removeInstance(this); closed = true; } diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java index 41f97622..c3f74f51 100644 --- a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java +++ b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminCore.java @@ -41,8 +41,6 @@ import org.apache.aries.rsa.util.EndpointHelper; import org.apache.aries.rsa.util.StringPlus; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; -import org.osgi.framework.ServiceEvent; -import org.osgi.framework.ServiceListener; import org.osgi.framework.ServiceReference; import org.osgi.framework.ServiceRegistration; import org.osgi.service.remoteserviceadmin.EndpointDescription; @@ -68,7 +66,6 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { private final BundleContext bctx; private final EventProducer eventProducer; - private ServiceListener exportedServiceListener; private DistributionProvider provider; private BundleContext apictx; private CloseHandler closeHandler; @@ -82,13 +79,21 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { this.eventProducer = eventProducer; this.provider = provider; this.closeHandler = new CloseHandler() { - public void onClose(ExportRegistration exportReg) { - removeExportRegistration(exportReg); - ExportReference exportReference = exportReg.getExportReference(); - if (exportReference != null) { - ServiceReference<?> serviceReference = exportReference.getExportedService(); - if (serviceReference != null) - getBundleContext(serviceReference).ungetService(serviceReference); + public void onClose(ExportRegistration reg) { + removeExportRegistration(reg); + if (reg.getException() != null) { + return; // there is no reference to close, and an exception if we try getting it + } + ExportReference ref = reg.getExportReference(); + ServiceReference<?> sref = ref == null ? null : ref.getExportedService(); + Bundle bundle = sref == null ? null : sref.getBundle(); + BundleContext context = bundle == null ? null : bundle.getBundleContext(); + // the bundle/context may already be closed, e.g. when called from + // the service factory ungetService when the bundle is stopped - + // the context is already invalid at that point, and services are + // automatically unregistered by the framework + if (context != null) { + context.ungetService(sref); } } @@ -96,17 +101,6 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { removeImportRegistration(importReg); } }; - createServiceListener(); - } - - // listen for exported services being unregistered, so we can close the export - protected void createServiceListener() { - this.exportedServiceListener = event -> { - if (event.getType() == ServiceEvent.UNREGISTERING) { - removeServiceExports(event.getServiceReference()); - } - }; - this.bctx.addServiceListener(exportedServiceListener); } @Override @@ -125,7 +119,7 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { } Map<String, Object> key = makeKey(serviceProperties); - List<ExportRegistration> regs = getExistingAndLock(key, interfaceNames); + List<ExportRegistration> regs = getExistingOrLock(key, interfaceNames); if (regs == null) { regs = new ArrayList<>(); try { @@ -177,7 +171,7 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { } } - private List<ExportRegistration> getExistingAndLock(Map<String, Object> key, List<String> interfaces) { + private List<ExportRegistration> getExistingOrLock(Map<String, Object> key, List<String> interfaces) { synchronized (exportedServices) { // check if it is already exported... Collection<ExportRegistration> regs = exportedServices.get(key); @@ -340,22 +334,17 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { // create a new list with copies of the exportRegistrations List<ExportRegistration> copy = new ArrayList<>(regs.size()); - for (ExportRegistration exportRegistration : regs) { - if (exportRegistration instanceof ExportRegistrationImpl) { - ExportRegistrationImpl exportRegistrationImpl = (ExportRegistrationImpl) exportRegistration; - if (exportRegistration.getException() == null) { - // Can only retrieve reference if we have no exception - EndpointDescription epd = exportRegistration.getExportReference().getExportedEndpoint(); - // create one copy for each distinct endpoint description - if (!copiedEndpoints.contains(epd)) { - copiedEndpoints.add(epd); - copy.add(new ExportRegistrationImpl(exportRegistrationImpl)); - // also increase service reference count - ServiceReference<?> serviceReference = exportRegistration.getExportReference().getExportedService(); - BundleContext serviceContext = getBundleContext(serviceReference); - serviceContext.getService(serviceReference); // unget it when export is closed - } - } + for (ExportRegistration reg : regs) { + ExportRegistrationImpl exportRegistrationImpl = (ExportRegistrationImpl) reg; + ExportReference ref = reg.getException() != null ? null : reg.getExportReference(); + EndpointDescription endpoint = ref == null ? null : ref.getExportedEndpoint(); + // create one copy for each distinct endpoint description + if (endpoint != null && copiedEndpoints.add(endpoint)) { + copy.add(new ExportRegistrationImpl(exportRegistrationImpl)); + // also increase service reference count + ServiceReference<?> sref = ref.getExportedService(); + BundleContext serviceContext = getBundleContext(sref); + serviceContext.getService(sref); // unget it when export is closed } } @@ -484,33 +473,6 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { return imReg; } - /** - * Removes and closes all exports for the given service. - * This is called when the service is unregistered. - * - * @param sref the service whose exports should be removed and closed - */ - protected void removeServiceExports(ServiceReference<?> sref) { - List<ExportRegistration> regs = new ArrayList<>(1); - synchronized (exportedServices) { - for (Collection<ExportRegistration> value : exportedServices.values()) { - for (ExportRegistration er : value) { - if (er.getException() != null || - er.getExportReference() == null || - er.getExportReference().getExportedService().equals(sref)) { - regs.add(er); - } - } - } - // do this outside of iteration to avoid concurrent modification - for (ExportRegistration er : regs) { - LOG.debug("closing export for service {}", sref); - er.close(); - } - } - - } - /** * Removes the provided Export Registration from the internal management structures. * This is called from the ExportRegistration itself when it is closed (so should @@ -539,45 +501,6 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { } } - // remove all export registrations associated with the given bundle - protected void removeExportRegistrations(Bundle exportingBundle) { - List<ExportRegistration> bundleExports = getExportsForBundle(exportingBundle); - for (ExportRegistration export : bundleExports) { - export.close(); - } - } - - // remove all import registrations - protected void closeImportRegistrations() { - Collection<ImportRegistration> copy = new ArrayList<>(); - synchronized (importedServices) { - for (Collection<ImportRegistration> irs : importedServices.values()) { - copy.addAll(irs); - } - } - for (ImportRegistration ir : copy) { - ir.close(); - } - } - - private List<ExportRegistration> getExportsForBundle(Bundle exportingBundle) { - synchronized (exportedServices) { - List<ExportRegistration> bundleRegs = new ArrayList<>(); - for (Collection<ExportRegistration> regs : exportedServices.values()) { - if (!regs.isEmpty()) { - ExportRegistration exportRegistration = regs.iterator().next(); - if (exportRegistration.getException() == null && exportRegistration.getExportReference() != null) { - Bundle regBundle = exportRegistration.getExportReference().getExportedService().getBundle(); - if (exportingBundle.equals(regBundle)) { - bundleRegs.addAll(regs); - } - } - } - } - return bundleRegs; - } - } - protected void removeImportRegistration(ImportRegistration iri) { synchronized (importedServices) { LOG.debug("Removing importRegistration {}", iri); @@ -599,14 +522,6 @@ public class RemoteServiceAdminCore implements RemoteServiceAdmin { } } - public void close() { - LOG.info("Closing {}", this.getClass().getSimpleName()); - closeImportRegistrations(); - if (exportedServiceListener != null) { - bctx.removeServiceListener(exportedServiceListener); - } - } - static void overlayProperties(Map<String, Object> serviceProperties, Map<String, Object> additionalProperties) { Map<String, String> keysLowerCase = new HashMap<>(); diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminFactory.java b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminFactory.java index fe4fcf72..92730573 100644 --- a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminFactory.java +++ b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminFactory.java @@ -25,27 +25,42 @@ import org.osgi.service.remoteserviceadmin.RemoteServiceAdmin; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * The {@link ServiceFactory} instance that supplies bundles each with their own RSA instance. + * <p> + * For every bundle, the first time it gets the service the factory's getService is invoked + * to create the instance, and then the framework caches the instance and returns it when + * the bundle subsequently calls get/unget while managing its reference count. Only when + * the reference count reaches zero (the last time unget is called) is the factory's + * ungetService invoked to destroy the instance. + * <p> + * We use a ServiceFactory according to the spec's recommendation, in order to implement + * the requirement that if a TopologyManager bundle is stopped, all registrations that + * were made by it must be closed, and when the RSA bundle is stopped all of its + * registrations must be closed as well. In both cases, when using a factory, our + * ungetService method will be invoked when either bundle is stopped, since the framework + * automatically ungets all remaining services when stopped. + */ public class RemoteServiceAdminFactory implements ServiceFactory<RemoteServiceAdmin> { private static final Logger LOG = LoggerFactory.getLogger(RemoteServiceAdminFactory.class); private final RemoteServiceAdminCore rsaCore; - private int instances; public RemoteServiceAdminFactory(RemoteServiceAdminCore rsaCore) { this.rsaCore = rsaCore; } public synchronized RemoteServiceAdmin getService(Bundle b, ServiceRegistration<RemoteServiceAdmin> sreg) { - LOG.debug("new RemoteServiceAdmin ServiceInstance created for Bundle {}", b.getSymbolicName()); - instances++; + LOG.debug("new RemoteServiceAdmin instance created for Bundle {}", b.getSymbolicName()); return new RemoteServiceAdminInstance(b.getBundleContext(), rsaCore); } public synchronized void ungetService(Bundle b, ServiceRegistration<RemoteServiceAdmin> sreg, RemoteServiceAdmin serviceObject) { - LOG.debug("RemoteServiceAdmin ServiceInstance removed for Bundle {}", b.getSymbolicName()); - instances--; - ((RemoteServiceAdminInstance)serviceObject).close(b, instances == 0); + LOG.debug("RemoteServiceAdmin instance destroyed for Bundle {}", b.getSymbolicName()); + // close all registrations that were created by the calling bundle + // (typically TopologyManager) when it is stopped, as required by the spec + ((RemoteServiceAdminInstance)serviceObject).close(); } } diff --git a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminInstance.java b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminInstance.java index 27aef96a..c7059483 100644 --- a/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminInstance.java +++ b/rsa/src/main/java/org/apache/aries/rsa/core/RemoteServiceAdminInstance.java @@ -21,11 +21,10 @@ package org.apache.aries.rsa.core; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; -import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; import org.osgi.framework.ServiceReference; @@ -37,54 +36,67 @@ import org.osgi.service.remoteserviceadmin.ImportReference; import org.osgi.service.remoteserviceadmin.ImportRegistration; import org.osgi.service.remoteserviceadmin.RemoteServiceAdmin; -public class RemoteServiceAdminInstance implements RemoteServiceAdmin { +/** + * An RSA instance that is created and returned by the {@link RemoteServiceAdminFactory}, + * one per calling bundle. All instances delegate the heavy lifting to a shared + * RSA core implementation, but each instance keeps track of the registrations that + * were performed through it so that it can later close them when the corresponding bundle + * (TopologyManager) is closed. + */ +public class RemoteServiceAdminInstance implements RemoteServiceAdmin, CloseHandler { - // Context of the bundle requesting the RemoteServiceAdmin - private final BundleContext bctx; + // Context of the bundle that is using this the RemoteServiceAdmin (i.e. the TopologyManager) + private final BundleContext context; private final RemoteServiceAdminCore rsaCore; + private final List<ExportRegistration> exportRegistrations = new CopyOnWriteArrayList<>(); + private final List<ImportRegistration> importRegistrations = new CopyOnWriteArrayList<>(); - private boolean closed; - - public RemoteServiceAdminInstance(BundleContext bc, RemoteServiceAdminCore core) { - bctx = bc; - rsaCore = core; + public RemoteServiceAdminInstance(BundleContext context, RemoteServiceAdminCore rsaCore) { + this.context = context; + this.rsaCore = rsaCore; } @Override - @SuppressWarnings("rawtypes") public List<ExportRegistration> exportService(final ServiceReference ref, final Map properties) { - return closed ? Collections.emptyList() : rsaCore.exportService(ref, properties); + List<ExportRegistration> regs = rsaCore.exportService(ref, properties); + // we need to keep track of our open registrations, and be notified when they are closed + regs.forEach(reg -> ((ExportRegistrationImpl)reg).addCloseHandler(this)); + exportRegistrations.addAll(regs); + return regs; } @Override public Collection<ExportReference> getExportedServices() { checkPermission(new EndpointPermission("*", EndpointPermission.READ)); - return closed ? null : rsaCore.getExportedServices(); + return rsaCore.getExportedServices(); } @Override public Collection<ImportReference> getImportedEndpoints() { checkPermission(new EndpointPermission("*", EndpointPermission.READ)); - return closed ? null : rsaCore.getImportedEndpoints(); + return rsaCore.getImportedEndpoints(); } @Override public ImportRegistration importService(final EndpointDescription endpoint) { - String frameworkUUID = bctx.getProperty(Constants.FRAMEWORK_UUID); + String frameworkUUID = context.getProperty(Constants.FRAMEWORK_UUID); checkPermission(new EndpointPermission(endpoint, frameworkUUID, EndpointPermission.IMPORT)); - return AccessController.doPrivileged(new PrivilegedAction<ImportRegistration>() { - public ImportRegistration run() { - return closed ? null : rsaCore.importService(endpoint); - } - }); + ImportRegistration reg = AccessController.doPrivileged((PrivilegedAction<ImportRegistration>) + () -> rsaCore.importService(endpoint)); + if (reg != null) { + // we need to keep track of our open registrations, and be notified when they are closed + ((ImportRegistrationImpl)reg).addCloseHandler(this); + importRegistrations.add(reg); + } + return reg; } - public void close(Bundle bundle, boolean closeAll) { - closed = true; - rsaCore.removeExportRegistrations(bundle); - if (closeAll) { - rsaCore.close(); - } + /** + * Close all registrations that were made through this RSA instance. + */ + public void close() { + importRegistrations.forEach(ImportRegistration::close); + exportRegistrations.forEach(ExportRegistration::close); } private void checkPermission(EndpointPermission permission) { @@ -93,4 +105,14 @@ public class RemoteServiceAdminInstance implements RemoteServiceAdmin { sm.checkPermission(permission); } } + + @Override + public void onClose(ExportRegistration reg) { + exportRegistrations.remove(reg); // registration was closed, no need to track it anymore + } + + @Override + public void onClose(ImportRegistration reg) { + importRegistrations.remove(reg); // registration was closed, no need to track it anymore + } } diff --git a/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java b/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java index 6e1fa8aa..2b10771d 100644 --- a/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java +++ b/rsa/src/test/java/org/apache/aries/rsa/core/DistributionProviderTrackerTest.java @@ -45,6 +45,7 @@ public class DistributionProviderTrackerTest { public void testAddingRemoved() throws InvalidSyntaxException { IMocksControl c = EasyMock.createControl(); DistributionProvider provider = c.createMock(DistributionProvider.class); + expect(provider.getSupportedTypes()).andReturn(new String[] { "config-type" }).anyTimes(); ServiceReference<DistributionProvider> providerRef = c.createMock(ServiceReference.class); expect(providerRef.getProperty(RemoteConstants.REMOTE_INTENTS_SUPPORTED)).andReturn(""); @@ -59,8 +60,6 @@ public class DistributionProviderTrackerTest { expect(context.registerService(EasyMock.isA(String.class), EasyMock.isA(ServiceFactory.class), EasyMock.isA(Dictionary.class))) .andReturn(rsaReg).atLeastOnce(); - context.addServiceListener(anyObject(ServiceListener.class)); - expectLastCall().anyTimes(); final BundleContext apiContext = c.createMock(BundleContext.class); c.replay(); @@ -69,7 +68,7 @@ public class DistributionProviderTrackerTest { return apiContext; } }; - tracker.addingService(providerRef); + ServiceRegistration<RemoteServiceAdminFactory> factoryRegistration = tracker.addingService(providerRef); c.verify(); c.reset(); @@ -77,7 +76,7 @@ public class DistributionProviderTrackerTest { EasyMock.expectLastCall(); EasyMock.expect(context.ungetService(providerRef)).andReturn(true); c.replay(); - tracker.removedService(providerRef, rsaReg); + tracker.removedService(providerRef, factoryRegistration); c.verify(); } @@ -85,6 +84,7 @@ public class DistributionProviderTrackerTest { public void testAddingWithNullValues() throws InvalidSyntaxException { IMocksControl c = EasyMock.createControl(); DistributionProvider provider = c.createMock(DistributionProvider.class); + expect(provider.getSupportedTypes()).andReturn(new String[] { "config-type" }).anyTimes(); ServiceReference<DistributionProvider> providerRef = c.createMock(ServiceReference.class); expect(providerRef.getProperty(RemoteConstants.REMOTE_INTENTS_SUPPORTED)).andReturn(null);
