cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From amic...@apache.org
Subject 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/
Date Thu, 15 Aug 2013 18:24:21 GMT
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(),



Mime
View raw message