Tidy up the Vaadin OSGi whiteboard component (#9648)

The Vaadin OSGi integration component uses Declarative Services, but it does some odd things:
 * It uses the whiteboard service's own bundle context to get hold of the service instance
 * It has an asymmetric get/release for the whiteboard service, which could leak instances over time
 * It releases service instances that it is still actively using

This change tidies up the service lifecycle by delegating the get/release to the Service Component Runtime managing the container (specifically by injecting the service instance). Using this injection also ensures that the Vaadin whiteboard service is obtained using this component's bundle context.

This change also simplifies the code a little by using the reference as the key to track the registrations. Different references for the same service are required to be equal so there is no issue with doing this.

This change does not alter the fact that the whiteboard service's bundle context is used to register the Http Whiteboard servlet, as this may be the intended behaviour. As a result the component should be prepared for an IllegalStateException when unregistering the service whiteboard service, which may already have been unregistered by the OSGi framework if the target bundle is stopping.

diff --git a/osgi-integration/pom.xml b/osgi-integration/pom.xml
index cebdf2f640d99715d47dc9123e8efc209a3a2717..e1ef1363ac36ad8c7259550e12095d4246e8be22 100644 (file)
--- a/osgi-integration/pom.xml
+++ b/osgi-integration/pom.xml
@@ -56,7 +56,6 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-compiler-plugin</artifactId>
-                <version>3.1</version>
             </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>

diff --git a/osgi-integration/src/main/java/com/vaadin/osgi/servlet/VaadinServletRegistration.java b/osgi-integration/src/main/java/com/vaadin/osgi/servlet/VaadinServletRegistration.java
index d7ec6289ce7bc2b4245a08c02fff709ce4aa30df..4ab1cf1505d918e161ab9c8e50f79ab493e540ad 100644 (file)
--- a/osgi-integration/src/main/java/com/vaadin/osgi/servlet/VaadinServletRegistration.java
+++ b/osgi-integration/src/main/java/com/vaadin/osgi/servlet/VaadinServletRegistration.java
@@ -48,9 +48,9 @@ import com.vaadin.server.VaadinServlet;
  *
  * @since 8.1
  */
-@Component(immediate = true)
+@Component
 public class VaadinServletRegistration {
-    private Map<Long, ServiceRegistration<?>> registeredServlets = Collections
+    private final Map<ServiceReference<VaadinServlet>, ServiceRegistration<Servlet>> registeredServlets = Collections
             .synchronizedMap(new LinkedHashMap<>());
 
     private static final String MISSING_ANNOTATION_MESSAGE_FORMAT = "The property '%s' must be set in a '%s' without the '%s' annotation!";
@@ -64,19 +64,18 @@ public class VaadinServletRegistration {
     private LogService logService;
 
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, service = VaadinServlet.class, policy = ReferencePolicy.DYNAMIC)
-    void bindVaadinServlet(ServiceReference<VaadinServlet> reference)
+    void bindVaadinServlet(VaadinServlet servlet, ServiceReference<VaadinServlet> reference)
             throws ResourceBundleInactiveException {
         log(LogService.LOG_WARNING, "VaadinServlet Registration");
-        BundleContext bundleContext = reference.getBundle().getBundleContext();
-        Hashtable<String, Object> properties = getProperties(reference);
 
-        VaadinServlet servlet = bundleContext.getService(reference);
+        Hashtable<String, Object> properties = getProperties(reference);
 
         WebServlet annotation = servlet.getClass()
                 .getAnnotation(WebServlet.class);
 
-        if (!validateSettings(annotation, properties))
+        if (!validateSettings(annotation, properties)) {
             return;
+        }
 
         properties.put(VAADIN_RESOURCES_PARAM, getResourcePath());
         if (annotation != null) {
@@ -85,17 +84,13 @@ public class VaadinServletRegistration {
                     Boolean.toString(annotation.asyncSupported()));
         }
 
+        // We register the Http Whiteboard servlet using the context of
+        // the bundle which registered the Vaadin Servlet, not our own
+        BundleContext bundleContext = reference.getBundle().getBundleContext();
         ServiceRegistration<Servlet> servletRegistration = bundleContext
                 .registerService(Servlet.class, servlet, properties);
-        Long serviceId = getServiceId(reference);
-        registeredServlets.put(serviceId, servletRegistration);
-
-        bundleContext.ungetService(reference);
-    }
 
-    private Long getServiceId(ServiceReference<VaadinServlet> reference) {
-        return (Long) reference
-                .getProperty(org.osgi.framework.Constants.SERVICE_ID);
+        registeredServlets.put(reference, servletRegistration);
     }
 
     private boolean validateSettings(WebServlet annotation,
@@ -128,12 +123,18 @@ public class VaadinServletRegistration {
         }
     }
 
-    void unbindVaadinServlet(ServiceReference<VaadinServlet> servletRef) {
-        Long serviceId = getServiceId(servletRef);
+    void unbindVaadinServlet(ServiceReference<VaadinServlet> reference) {
         ServiceRegistration<?> servletRegistration = registeredServlets
-                .remove(serviceId);
+                .remove(reference);
         if (servletRegistration != null) {
-            servletRegistration.unregister();
+            try {
+                servletRegistration.unregister();
+            } catch (IllegalStateException ise) {
+                // This service may have already been unregistered
+                // automatically by the OSGi framework if the
+                // application bundle is being stopped. This is
+                // obviously not a problem for us.
+            }
         }
     }