svn commit: r1727036 - /sling/trunk/bundles/extensions/discovery/oak/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java

[st...@apache.org]

Author: stefanegli
Date: Wed Jan 27 13:21:26 2016
New Revision: 1727036

URL: http://svn.apache.org/viewvc?rev=1727036&view=rev
Log:
SLING-5382 : avoid NPE when PropertyProviders are bind/unbind/changed before activate

Modified:
    sling/trunk/bundles/extensions/discovery/oak/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java

Modified: sling/trunk/bundles/extensions/discovery/oak/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/discovery/oak/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java?rev=1727036&r1=1727035&r2=1727036&view=diff
==============================================================================
--- sling/trunk/bundles/extensions/discovery/oak/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java (original)
+++ sling/trunk/bundles/extensions/discovery/oak/src/main/java/org/apache/sling/discovery/oak/OakDiscoveryService.java Wed Jan 27 13:21:26 2016
@@ -107,7 +107,7 @@ public class OakDiscoveryService extends
      * whether or not this service is activated - necessary to avoid sending
      * events to discovery awares before activate is done
      **/
-    private boolean activated = false;
+    private volatile boolean activated = false;
 
     @Reference
     private ResourceResolverFactory resourceResolverFactory;
@@ -400,7 +400,9 @@ public class OakDiscoveryService extends
         final ProviderInfo info = new ProviderInfo(propertyProvider, props);
         this.providerInfos.add(info);
         Collections.sort(this.providerInfos);
-        this.doUpdateProperties();
+        if (activated) {
+            this.doUpdateProperties();
+        }
         checkForTopologyChange();
     }
 
@@ -439,7 +441,9 @@ public class OakDiscoveryService extends
 
         final ProviderInfo info = new ProviderInfo(propertyProvider, props);
         if ( this.providerInfos.remove(info) && update ) {
-            this.doUpdateProperties();
+            if (activated) {
+                this.doUpdateProperties();
+            }
             this.checkForTopologyChange();
         }
     }
@@ -454,9 +458,19 @@ public class OakDiscoveryService extends
      * @see Config#getClusterInstancesPath()
      */
     private void doUpdateProperties() {
-        if (resourceResolverFactory == null) {
+        // SLING-5382 : the caller must ensure that this method
+        // is not invoked after deactivation or before activation.
+        // so this method doesn't have to do any further synchronization.
+        // what we do nevertheless is a paranoia way of checking if
+        // all variables are available and do a NOOP if that's not the case.
+        final ResourceResolverFactory rrf = resourceResolverFactory;
+        final Config c = config;
+        final String sid = slingId;
+        if (rrf == null || c == null || sid == null) {
             // cannot update the properties then..
-            logger.debug("doUpdateProperties: too early to update the properties. resourceResolverFactory not yet set.");
+            logger.debug("doUpdateProperties: too early to update the properties. "
+                    + "resourceResolverFactory ({}), config ({}) or slingId ({}) not yet set.",
+                    new Object[] {rrf, c, sid});
             return;
         } else {
             logger.debug("doUpdateProperties: updating properties now..");
@@ -470,14 +484,14 @@ public class OakDiscoveryService extends
 
         ResourceResolver resourceResolver = null;
         try {
-            resourceResolver = resourceResolverFactory
+            resourceResolver = rrf
                     .getAdministrativeResourceResolver(null);
 
             Resource myInstance = ResourceHelper
                     .getOrCreateResource(
                             resourceResolver,
-                            config.getClusterInstancesPath()
-                                    + "/" + slingId + "/properties");
+                            c.getClusterInstancesPath()
+                                    + "/" + sid + "/properties");
             // SLING-2879 - revert/refresh resourceResolver here to work
             // around a potential issue with jackrabbit in a clustered environment
             resourceResolver.revert();
@@ -541,8 +555,12 @@ public class OakDiscoveryService extends
      */
     public void updateProperties() {
         synchronized (lock) {
-            logger.debug("updateProperties: calling doUpdateProperties.");
-            doUpdateProperties();
+            if (!activated) {
+                logger.debug("updateProperties: not yet activated, not calling doUpdateProperties this time.");
+            } else {
+                logger.debug("updateProperties: calling doUpdateProperties.");
+                doUpdateProperties();
+            }
             logger.debug("updateProperties: calling handlePotentialTopologyChange.");
             checkForTopologyChange();
             logger.debug("updateProperties: done.");