You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by am...@apache.org on 2013/08/15 20:24:21 UTC
svn commit: r1514424 - in /cxf/dosgi/trunk/dsw/cxf-dsw/src:
main/java/org/apache/cxf/dosgi/dsw/service/
test/java/org/apache/cxf/dosgi/dsw/service/
Author: amichai
Date: Thu Aug 15 18:24:21 2013
New Revision: 1514424
URL: http://svn.apache.org/r1514424
Log:
Refactored/simplified ExportRegistrationImpl, fixed synchronization, replaced ServiceTracker with ServiceListener fixing occasional reference leak when failed export is closed
Modified:
cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/ExportRegistrationImpl.java
cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java
cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminInstance.java
cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/EventProducerTest.java
cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java
Modified: cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/ExportRegistrationImpl.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/ExportRegistrationImpl.java?rev=1514424&r1=1514423&r2=1514424&view=diff
==============================================================================
--- cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/ExportRegistrationImpl.java (original)
+++ cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/ExportRegistrationImpl.java Thu Aug 15 18:24:21 2013
@@ -23,16 +23,10 @@ import java.util.Map;
import java.util.Set;
import org.apache.cxf.endpoint.Server;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.Constants;
-import org.osgi.framework.Filter;
-import org.osgi.framework.InvalidSyntaxException;
import org.osgi.framework.ServiceReference;
import org.osgi.service.remoteserviceadmin.EndpointDescription;
import org.osgi.service.remoteserviceadmin.ExportReference;
import org.osgi.service.remoteserviceadmin.ExportRegistration;
-import org.osgi.util.tracker.ServiceTracker;
-import org.osgi.util.tracker.ServiceTrackerCustomizer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -40,73 +34,86 @@ public class ExportRegistrationImpl impl
private static final Logger LOG = LoggerFactory.getLogger(ExportRegistrationImpl.class);
- private Server server;
- private boolean closed;
- private Throwable exception;
-
- private ExportRegistrationImpl parent;
- private volatile int instanceCount = 1;
-
- private RemoteServiceAdminCore rsaCore;
-
- private ExportReferenceImpl exportReference;
-
- private ServiceTracker serviceTracker;
-
- // provide a clone of the provided exp.Reg that is linked to this instance
- public ExportRegistrationImpl(ExportRegistrationImpl exportRegistration) {
- parent = exportRegistration;
- exportReference = new ExportReferenceImpl(parent.getExportReference());
- exception = parent.getException();
- rsaCore = parent.getRsaCore();
- parent.instanceAdded();
+ private final RemoteServiceAdminCore rsaCore;
+ private final ExportReferenceImpl exportReference;
+ private final Server server;
+ private final Throwable exception;
+
+ private final ExportRegistrationImpl parent;
+ private int instanceCount;
+ private volatile boolean closed;
+
+ private ExportRegistrationImpl(ExportRegistrationImpl parent, RemoteServiceAdminCore rsaCore,
+ ExportReferenceImpl exportReference, Server server, Throwable exception) {
+ this.parent = parent != null ? parent.parent : this; // a parent points to itself
+ this.parent.addInstance();
+ this.rsaCore = rsaCore;
+ this.exportReference = exportReference;
+ this.server = server;
+ this.exception = exception;
+ }
+
+ // create a clone of the provided ExportRegistrationImpl that is linked to it
+ public ExportRegistrationImpl(ExportRegistrationImpl parent) {
+ this(parent, parent.rsaCore, new ExportReferenceImpl(parent.exportReference),
+ parent.server, parent.exception);
}
+ // create a new (parent) instance which was exported successfully with the given server
public ExportRegistrationImpl(ServiceReference sref, EndpointDescription endpoint,
- RemoteServiceAdminCore remoteServiceAdminCore) {
- exportReference = new ExportReferenceImpl(sref, endpoint);
- parent = this;
- rsaCore = remoteServiceAdminCore;
+ RemoteServiceAdminCore rsaCore, Server server) {
+ this(null, rsaCore, new ExportReferenceImpl(sref, endpoint), server, null);
}
- private void instanceAdded() {
- ++instanceCount;
+ // create a new (parent) instance which failed to be exported with the given exception
+ public ExportRegistrationImpl(ServiceReference sref, EndpointDescription endpoint,
+ RemoteServiceAdminCore rsaCore, Throwable exception) {
+ this(null, rsaCore, new ExportReferenceImpl(sref, endpoint), null, exception);
}
- public synchronized void close() {
- if (closed) {
- return;
+ private void ensureParent() {
+ if (parent != this) {
+ throw new IllegalStateException("this method may only be called on the parent");
}
- closed = true;
+ }
- rsaCore.removeExportRegistration(this);
+ public ExportReference getExportReference() {
+ return closed ? null : exportReference;
+ }
- parent.instanceClosed();
- if (server != null) {
- server.stop();
- server = null;
+ public Throwable getException() {
+ return closed ? null : exception;
+ }
+
+ public final void close() {
+ synchronized (this) {
+ if (closed) {
+ return;
+ }
+ closed = true;
}
+
+ rsaCore.removeExportRegistration(this);
exportReference.close();
+ parent.removeInstance();
+ }
- if (serviceTracker != null) {
- serviceTracker.close();
- serviceTracker = null;
+ private void addInstance() {
+ ensureParent();
+ synchronized (this) {
+ instanceCount++;
}
}
- private void instanceClosed() {
- --instanceCount;
- if (instanceCount <= 0) {
- // really close the ExReg
- // TODO close it and remove from management structure!
-
- LOG.debug("really closing ExportRegistration now!");
+ private void removeInstance() {
+ ensureParent();
+ synchronized (this) {
+ instanceCount--;
+ if (instanceCount <= 0) {
+ LOG.debug("really closing ExportRegistration now!");
- synchronized (this) {
if (server != null) {
- // FIXME: is this done like this?
- server.stop();
- server = null;
+ server.destroy();
}
}
}
@@ -114,15 +121,14 @@ public class ExportRegistrationImpl impl
@Override
public String toString() {
- if (exportReference == null) {
- return "Exportregistration closed";
+ if (closed) {
+ return "ExportRegistration closed";
}
EndpointDescription endpoint = getExportReference().getExportedEndpoint();
ServiceReference serviceReference = getExportReference().getExportedService();
String r = "EndpointDescription for ServiceReference " + serviceReference;
- r += "\n";
- r += "*** EndpointDescription: **** \n";
+ r += "\n*** EndpointDescription: ****\n";
if (endpoint == null) {
r += "---> NULL <---- \n";
} else {
@@ -135,70 +141,4 @@ public class ExportRegistrationImpl impl
}
return r;
}
-
- public Throwable getException() {
- return closed ? null : exception;
- }
-
- public void setServer(Server server) {
- this.server = server;
- }
-
- public void setException(Throwable ex) {
- exception = ex;
- }
-
- public ExportReference getExportReference() {
- return exportReference;
- }
-
- public RemoteServiceAdminCore getRsaCore() {
- return rsaCore;
- }
-
- /**
- * Start the service tracker that monitors the osgi service that
- * is exported by this exportRegistration
- * */
- public void startServiceTracker(BundleContext bctx) {
- // only the parent should do this
- if (parent != this) {
- parent.startServiceTracker(bctx);
- return;
- }
-
- // do it only once
- if (serviceTracker != null) {
- return;
- }
-
- Filter f;
- final Long sid = (Long)getExportReference().getExportedService().getProperty(Constants.SERVICE_ID);
- try {
- f = bctx.createFilter("(" + Constants.SERVICE_ID + "=" + sid + ")");
- } catch (InvalidSyntaxException e) {
- LOG.warn("Service tracker could not be started. The service will not be automatically unexported "
- + e.getMessage(), e);
- return;
- }
- serviceTracker = new ServiceTracker(bctx, f, new ServiceTrackerCustomizer() {
-
- public void removedService(ServiceReference sr, Object s) {
- LOG.info("Service [" + sid + "] has been unregistered: Removing service export");
- close();
- }
-
- public void modifiedService(ServiceReference sr, Object s) {
- // FIXME:
- LOG.warn("Service modifications after the service is exported are "
- + "currently not supported. The export is not modified!");
- }
-
- public Object addingService(ServiceReference sr) {
- return sr;
- }
- });
- serviceTracker.open();
- }
-
}
Modified: cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java?rev=1514424&r1=1514423&r2=1514424&view=diff
==============================================================================
--- cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java (original)
+++ cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCore.java Thu Aug 15 18:24:21 2013
@@ -40,6 +40,9 @@ import org.apache.cxf.dosgi.dsw.util.Osg
import org.apache.cxf.dosgi.dsw.util.Utils;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
+import org.osgi.framework.InvalidSyntaxException;
+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;
@@ -67,11 +70,26 @@ public class RemoteServiceAdminCore impl
private final BundleContext bctx;
private final EventProducer eventProducer;
private final ConfigTypeHandlerFactory configTypeHandlerFactory;
+ private final ServiceListener exportedServiceListener;
public RemoteServiceAdminCore(BundleContext bc, ConfigTypeHandlerFactory configTypeHandlerFactory) {
this.bctx = bc;
this.eventProducer = new EventProducer(bctx);
this.configTypeHandlerFactory = configTypeHandlerFactory;
+ // listen for exported services being unregistered so we can close the export
+ this.exportedServiceListener = new ServiceListener() {
+ public void serviceChanged(ServiceEvent event) {
+ if (event.getType() == ServiceEvent.UNREGISTERING) {
+ removeServiceExports(event.getServiceReference());
+ }
+ }
+ };
+ try {
+ String filter = "(" + RemoteConstants.SERVICE_EXPORTED_INTERFACES + "=*)";
+ bc.addServiceListener(exportedServiceListener, filter);
+ } catch (InvalidSyntaxException ise) {
+ throw new RuntimeException(ise); // can never happen
+ }
}
@Override
@@ -166,15 +184,15 @@ public class RemoteServiceAdminCore impl
ExportResult exportResult = handler.createServer(serviceReference, bctx, bundle.getBundleContext(),
serviceProperties, interfaceClass, service);
EndpointDescription endpoint = new EndpointDescription(exportResult.getEndpointProps());
- ExportRegistrationImpl exportRegistration = new ExportRegistrationImpl(
- serviceReference, endpoint, this);
+ ExportRegistrationImpl exportRegistration;
if (exportResult.getException() == null) {
LOG.info("created server for interface " + iface);
- exportRegistration.setServer(exportResult.getServer());
- exportRegistration.startServiceTracker(bctx);
+ exportRegistration = new ExportRegistrationImpl(serviceReference, endpoint, this,
+ exportResult.getServer());
} else {
LOG.error("failed to create server for interface " + iface, exportResult.getException());
- exportRegistration.setException(exportResult.getException());
+ exportRegistration = new ExportRegistrationImpl(serviceReference, endpoint, this,
+ exportResult.getException());
}
exportRegs.add(exportRegistration);
}
@@ -387,8 +405,38 @@ public class RemoteServiceAdminCore impl
}
/**
- * Removes the provided Export Registration from the internal management structures -> intended to be used
- * when the export Registration is closed
+ * 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<ExportRegistration>(1);
+ synchronized (exportedServices) {
+ for (Iterator<Collection<ExportRegistration>> it = exportedServices.values().iterator(); it.hasNext();) {
+ Collection<ExportRegistration> value = it.next();
+ for (Iterator<ExportRegistration> it2 = value.iterator(); it2.hasNext();) {
+ ExportRegistration er = it2.next();
+ if (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
+ * not attempt to close it again here).
+ *
+ * @param eri the export registration to remove
*/
protected void removeExportRegistration(ExportRegistrationImpl eri) {
synchronized (exportedServices) {
@@ -459,4 +507,9 @@ public class RemoteServiceAdminCore impl
}
}
}
+
+ public void close() {
+ removeImportRegistrations();
+ bctx.removeServiceListener(exportedServiceListener);
+ }
}
Modified: cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminInstance.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminInstance.java?rev=1514424&r1=1514423&r2=1514424&view=diff
==============================================================================
--- cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminInstance.java (original)
+++ cxf/dosgi/trunk/dsw/cxf-dsw/src/main/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminInstance.java Thu Aug 15 18:24:21 2013
@@ -87,7 +87,7 @@ public class RemoteServiceAdminInstance
closed = true;
rsaCore.removeExportRegistrations(bctx.getBundle());
if (closeAll) {
- rsaCore.removeImportRegistrations();
+ rsaCore.close();
}
}
}
Modified: cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/EventProducerTest.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/EventProducerTest.java?rev=1514424&r1=1514423&r2=1514424&view=diff
==============================================================================
--- cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/EventProducerTest.java (original)
+++ cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/EventProducerTest.java Thu Aug 15 18:24:21 2013
@@ -24,6 +24,7 @@ import java.util.Hashtable;
import java.util.List;
import java.util.UUID;
+import org.apache.cxf.endpoint.Server;
import org.easymock.IAnswer;
import org.easymock.classextension.EasyMock;
import org.junit.Assert;
@@ -113,14 +114,14 @@ public class EventProducerTest {
EasyMock.replay(bc);
EventProducer eventProducer = new EventProducer(bc);
- ExportRegistrationImpl ereg = new ExportRegistrationImpl(sref, endpoint, remoteServiceAdminCore);
+ ExportRegistrationImpl ereg = new ExportRegistrationImpl(sref, endpoint, remoteServiceAdminCore, (Server)null);
eventProducer.publishNotification(ereg);
}
@Test
public void testPublishErrorNotification() throws Exception {
- RemoteServiceAdminCore remoteServiceAdminCore = EasyMock.createNiceMock(RemoteServiceAdminCore.class);
- EasyMock.replay(remoteServiceAdminCore);
+ RemoteServiceAdminCore rsaCore = EasyMock.createNiceMock(RemoteServiceAdminCore.class);
+ EasyMock.replay(rsaCore);
final EndpointDescription endpoint = EasyMock.createNiceMock(EndpointDescription.class);
EasyMock.expect(endpoint.getInterfaces()).andReturn(Arrays.asList("org.foo.Bar")).anyTimes();
@@ -177,8 +178,7 @@ public class EventProducerTest {
EasyMock.replay(bc);
EventProducer eventProducer = new EventProducer(bc);
- ExportRegistrationImpl ereg = new ExportRegistrationImpl(sref, endpoint, remoteServiceAdminCore);
- ereg.setException(exportException);
+ ExportRegistrationImpl ereg = new ExportRegistrationImpl(sref, endpoint, rsaCore, exportException);
eventProducer.publishNotification(Arrays.<ExportRegistration>asList(ereg));
}
}
Modified: cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java
URL: http://svn.apache.org/viewvc/cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java?rev=1514424&r1=1514423&r2=1514424&view=diff
==============================================================================
--- cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java (original)
+++ cxf/dosgi/trunk/dsw/cxf-dsw/src/test/java/org/apache/cxf/dosgi/dsw/service/RemoteServiceAdminCoreTest.java Thu Aug 15 18:24:21 2013
@@ -44,6 +44,7 @@ import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.FrameworkUtil;
+import org.osgi.framework.InvalidSyntaxException;
import org.osgi.framework.ServiceListener;
import org.osgi.framework.ServiceReference;
import org.osgi.service.remoteserviceadmin.EndpointDescription;
@@ -60,12 +61,16 @@ import static org.junit.Assert.assertTru
public class RemoteServiceAdminCoreTest {
@Test
- public void testDontExportOwnServiceProxies() {
+ public void testDontExportOwnServiceProxies() throws InvalidSyntaxException {
IMocksControl c = EasyMock.createControl();
Bundle b = c.createMock(Bundle.class);
BundleContext bc = c.createMock(BundleContext.class);
EasyMock.expect(bc.getBundle()).andReturn(b).anyTimes();
+ bc.addServiceListener(EasyMock.<ServiceListener>anyObject(), EasyMock.<String>anyObject());
+ EasyMock.expectLastCall().anyTimes();
+ bc.removeServiceListener(EasyMock.<ServiceListener>anyObject());
+ EasyMock.expectLastCall().anyTimes();
Dictionary<String, String> d = new Hashtable<String, String>();
EasyMock.expect(b.getHeaders()).andReturn(d).anyTimes();
@@ -172,6 +177,8 @@ public class RemoteServiceAdminCoreTest
EasyMock.expect(bc.getProperty(Constants.FRAMEWORK_VERSION)).andReturn(null).anyTimes();
bc.addServiceListener(EasyMock.<ServiceListener>anyObject(), EasyMock.<String>anyObject());
EasyMock.expectLastCall().anyTimes();
+ bc.removeServiceListener(EasyMock.<ServiceListener>anyObject());
+ EasyMock.expectLastCall().anyTimes();
EasyMock.expect(bc.getServiceReferences(EasyMock.<String>anyObject(),
EasyMock.<String>anyObject())).andReturn(null).anyTimes();
EasyMock.expect(bc.getAllServiceReferences(EasyMock.<String>anyObject(),