You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2016/03/17 13:56:27 UTC

svn commit: r1735417 - in /sling/trunk/bundles/resourceresolver/src: main/java/org/apache/sling/resourceresolver/impl/ main/java/org/apache/sling/resourceresolver/impl/providers/ test/java/org/apache/sling/resourceresolver/impl/providers/

Author: cziegeler
Date: Thu Mar 17 12:56:26 2016
New Revision: 1735417

URL: http://svn.apache.org/viewvc?rev=1735417&view=rev
Log:
SLING-5602 : The Discovery module does not work any more after a ResourceResolverFactory reactivation

Modified:
    sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/FactoryPreconditions.java
    sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
    sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java

Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/FactoryPreconditions.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/FactoryPreconditions.java?rev=1735417&r1=1735416&r2=1735417&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/FactoryPreconditions.java (original)
+++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/FactoryPreconditions.java Thu Mar 17 12:56:26 2016
@@ -47,38 +47,42 @@ public class FactoryPreconditions {
 
     private volatile List<RequiredProvider> requiredProviders;
 
-    public void activate(final BundleContext bc, final String[] configuration, ResourceProviderTracker tracker) {
-        this.tracker = tracker;
+    public void activate(final BundleContext bc, final String[] configuration, final ResourceProviderTracker tracker) {
+        synchronized ( this ) {
+            this.tracker = tracker;
 
-        final List<RequiredProvider> rps = new ArrayList<RequiredProvider>();
-        if ( configuration != null ) {
-            final Logger logger = LoggerFactory.getLogger(getClass());
-            for(final String r : configuration) {
-                if ( r != null && r.trim().length() > 0 ) {
-                    final String value = r.trim();
-                    RequiredProvider rp = new RequiredProvider();
-                    if ( value.startsWith("(") ) {
-                        try {
-                            rp.filter = bc.createFilter(value);
-                        } catch (final InvalidSyntaxException e) {
-                            logger.warn("Ignoring invalid filter syntax for required provider: " + value, e);
-                            rp = null;
+            final List<RequiredProvider> rps = new ArrayList<RequiredProvider>();
+            if ( configuration != null ) {
+                final Logger logger = LoggerFactory.getLogger(getClass());
+                for(final String r : configuration) {
+                    if ( r != null && r.trim().length() > 0 ) {
+                        final String value = r.trim();
+                        RequiredProvider rp = new RequiredProvider();
+                        if ( value.startsWith("(") ) {
+                            try {
+                                rp.filter = bc.createFilter(value);
+                            } catch (final InvalidSyntaxException e) {
+                                logger.warn("Ignoring invalid filter syntax for required provider: " + value, e);
+                                rp = null;
+                            }
+                        } else {
+                            rp.pid = value;
+                        }
+                        if ( rp != null ) {
+                            rps.add(rp);
                         }
-                    } else {
-                        rp.pid = value;
-                    }
-                    if ( rp != null ) {
-                        rps.add(rp);
                     }
                 }
             }
+            this.requiredProviders = rps;
         }
-        this.requiredProviders = rps;
     }
 
     public void deactivate() {
-        this.requiredProviders = null;
-        this.tracker = null;
+        synchronized ( this ) {
+            this.requiredProviders = null;
+            this.tracker = null;
+        }
     }
 
     public boolean checkPreconditions(final String unavailableServicePid) {

Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java?rev=1735417&r1=1735416&r2=1735417&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java (original)
+++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java Thu Mar 17 12:56:26 2016
@@ -76,7 +76,7 @@ import org.osgi.service.event.EventAdmin
 @References({
     @Reference(name = "ResourceDecorator", referenceInterface = ResourceDecorator.class, cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE, policy = ReferencePolicy.DYNAMIC)
 })
-public class ResourceResolverFactoryActivator implements Runnable {
+public class ResourceResolverFactoryActivator {
 
     private static final class FactoryRegistration {
         /** Registration .*/
@@ -257,22 +257,25 @@ public class ResourceResolverFactoryActi
     private final ResourceDecoratorTracker resourceDecoratorTracker = new ResourceDecoratorTracker();
 
     /** all mappings */
-    private Mapping[] mappings;
+    private volatile Mapping[] mappings;
 
     /** The fake URLs */
-    private BidiMap virtualURLMap;
+    private volatile BidiMap virtualURLMap;
 
     /** <code>true</code>, if direct mappings from URI to handle are allowed */
-    private boolean allowDirect = false;
+    private volatile boolean allowDirect = false;
 
     /** the search path for ResourceResolver.getResource(String) */
-    private String[] searchPath;
+    private volatile String[] searchPath;
 
     /** the root location of the /etc/map entries */
-    private String mapRoot;
+    private volatile String mapRoot;
 
     /** whether to mangle paths with namespaces or not */
-    private boolean mangleNamespacePrefixes;
+    private volatile boolean mangleNamespacePrefixes;
+
+    /** paranoid provider handling. */
+    private volatile boolean paranoidProviderHandling;
 
     /** Event admin. */
     @Reference
@@ -292,54 +295,40 @@ public class ResourceResolverFactoryActi
     /** ComponentContext */
     private volatile ComponentContext componentContext;
 
-    private int defaultVanityPathRedirectStatus;
+    private volatile int defaultVanityPathRedirectStatus;
 
     /** vanityPath enabled? */
-    private boolean enableVanityPath = DEFAULT_ENABLE_VANITY_PATH;
+    private volatile boolean enableVanityPath = DEFAULT_ENABLE_VANITY_PATH;
 
     /** alias resource resolution optimization enabled? */
-    private boolean enableOptimizeAliasResolution = DEFAULT_ENABLE_OPTIMIZE_ALIAS_RESOLUTION;
+    private volatile boolean enableOptimizeAliasResolution = DEFAULT_ENABLE_OPTIMIZE_ALIAS_RESOLUTION;
 
     /** max number of cache vanity path entries */
-    private long maxCachedVanityPathEntries = DEFAULT_MAX_CACHED_VANITY_PATHS;
+    private volatile long maxCachedVanityPathEntries = DEFAULT_MAX_CACHED_VANITY_PATHS;
 
     /** limit max number of cache vanity path entries only at startup*/
-    private boolean maxCachedVanityPathEntriesStartup = DEFAULT_MAX_CACHED_VANITY_PATHS_STARTUP;
+    private volatile boolean maxCachedVanityPathEntriesStartup = DEFAULT_MAX_CACHED_VANITY_PATHS_STARTUP;
 
     /** Maximum number of vanity bloom filter bytes */
-    private int vanityBloomFilterMaxBytes = DEFAULT_VANITY_BLOOM_FILTER_MAX_BYTES;
+    private volatile int vanityBloomFilterMaxBytes = DEFAULT_VANITY_BLOOM_FILTER_MAX_BYTES;
 
     /** vanity paths will have precedence over existing /etc/map mapping? */
-    private boolean vanityPathPrecedence = DEFAULT_VANITY_PATH_PRECEDENCE;
+    private volatile boolean vanityPathPrecedence = DEFAULT_VANITY_PATH_PRECEDENCE;
 
     /** log the place where a resource resolver is closed */
-    private boolean logResourceResolverClosing = DEFAULT_LOG_RESOURCE_RESOLVER_CLOSING;
+    private volatile boolean logResourceResolverClosing = DEFAULT_LOG_RESOURCE_RESOLVER_CLOSING;
 
     /** Vanity path whitelist */
-    private String[] vanityPathWhiteList;
+    private volatile String[] vanityPathWhiteList;
 
     /** Vanity path blacklist */
-    private String[] vanityPathBlackList;
+    private volatile String[] vanityPathBlackList;
 
     private final FactoryPreconditions preconds = new FactoryPreconditions();
 
     /** Factory registration. */
     private volatile FactoryRegistration factoryRegistration;
 
-    /** Background thread coordination. */
-    private final Object coordinator = new Object();
-
-    /** Background thread operation */
-    private enum BG_OP {
-        CHECK,                // check preconditions
-        UNREGISTER_AND_CHECK, // unregister first, then check preconditions
-        STOP                  // stop
-    }
-
-    private final List<BG_OP> operations = new ArrayList<BG_OP>();
-
-    private String[] requiredResourceProviders;
-
     /**
      * Get the resource decorator tracker.
      */
@@ -495,8 +484,11 @@ public class ResourceResolverFactoryActi
                         }
 
                         @Override
-                        public void providerRemoved(final String pid) {
+                        public void providerRemoved(final String pid, final boolean stateful) {
                             if ( factoryRegistration != null ) {
+                                if ( stateful || paranoidProviderHandling ) {
+                                    unregisterFactory();
+                                }
                                 checkFactoryPreconditions(pid);
                             }
                         }
@@ -557,18 +549,15 @@ public class ResourceResolverFactoryActi
         this.vanityPathPrecedence = PropertiesUtil.toBoolean(properties.get(PROP_VANITY_PATH_PRECEDENCE), DEFAULT_VANITY_PATH_PRECEDENCE);
         this.logResourceResolverClosing = PropertiesUtil.toBoolean(properties.get(PROP_LOG_RESOURCE_RESOLVER_CLOSING),
             DEFAULT_LOG_RESOURCE_RESOLVER_CLOSING);
+        this.paranoidProviderHandling = PropertiesUtil.toBoolean(properties.get(PROP_PARANOID_PROVIDER_HANDLING), DEFAULT_PARANOID_PROVIDER_HANDLING);
 
         final BundleContext bc = componentContext.getBundleContext();
 
         // check for required property
-        requiredResourceProviders = PropertiesUtil.toStringArray(properties.get(PROP_REQUIRED_PROVIDERS));
+        final String[] requiredResourceProviders = PropertiesUtil.toStringArray(properties.get(PROP_REQUIRED_PROVIDERS));
         this.preconds.activate(bc, requiredResourceProviders, resourceProviderTracker);
 
         this.checkFactoryPreconditions(null);
-
-        final Thread t = new Thread(this);
-        t.setDaemon(true);
-        t.start();
     }
 
     /**
@@ -576,14 +565,14 @@ public class ResourceResolverFactoryActi
      */
     @Deactivate
     protected void deactivate() {
+        this.componentContext = null;
+
+        this.unregisterFactory();
+
         this.changeListenerWhiteboard.deactivate();
         this.resourceProviderTracker.deactivate();
-        this.componentContext = null;
         this.preconds.deactivate();
         this.resourceDecoratorTracker.close();
-
-        this.unregisterFactory();
-        this.addOperation(BG_OP.STOP);
     }
 
     /**
@@ -690,58 +679,6 @@ public class ResourceResolverFactoryActi
         this.resourceDecoratorTracker.unbindResourceDecorator(decorator, props);
     }
 
-    @Override
-    public void run() {
-        boolean isRunning = true;
-        while ( isRunning ) {
-            final List<BG_OP> ops = new ArrayList<BG_OP>();
-            synchronized ( this.coordinator ) {
-                while ( this.operations.isEmpty() ) {
-                    try {
-                        this.coordinator.wait();
-                    } catch ( final InterruptedException ie ) {
-                        Thread.currentThread().interrupt();
-                    }
-                }
-                ops.addAll(this.operations);
-                this.operations.clear();
-            }
-            isRunning = !ops.contains(BG_OP.STOP);
-
-            if ( isRunning && ops.lastIndexOf(BG_OP.UNREGISTER_AND_CHECK) != -1 ) {
-                this.unregisterFactory();
-                // we only delay between unregister and check
-                // the delay is not really necessary but it provides
-                // a smoother unregistration/registration cascade
-                // and delaying for 2 secs should not hurt
-                try {
-                    Thread.sleep(2000);
-                } catch ( final InterruptedException ie ) {
-                    Thread.currentThread().interrupt();
-                }
-                // check for new operations and simply ignore them (except for STOP)
-                ops.clear();
-                synchronized ( this.coordinator ) {
-                    ops.addAll(this.operations);
-                    this.operations.clear();
-                }
-                isRunning = !ops.contains(BG_OP.STOP);
-            }
-
-            if ( isRunning ) {
-                this.checkFactoryPreconditions(null);
-            }
-        }
-        this.unregisterFactory();
-    }
-
-    private void addOperation(final BG_OP op) {
-        synchronized ( this.coordinator ) {
-            this.operations.add(op);
-            this.coordinator.notify();
-        }
-    }
-
     public ResourceProviderTracker getResourceProviderTracker() {
         return resourceProviderTracker;
     }

Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java?rev=1735417&r1=1735416&r2=1735417&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java (original)
+++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java Thu Mar 17 12:56:26 2016
@@ -35,6 +35,7 @@ import org.apache.sling.api.resource.obs
 import org.apache.sling.api.resource.observation.ResourceChange.ChangeType;
 import org.apache.sling.api.resource.path.Path;
 import org.apache.sling.api.resource.path.PathSet;
+import org.apache.sling.api.resource.runtime.dto.AuthType;
 import org.apache.sling.api.resource.runtime.dto.FailureReason;
 import org.apache.sling.api.resource.runtime.dto.ResourceProviderDTO;
 import org.apache.sling.api.resource.runtime.dto.ResourceProviderFailureDTO;
@@ -68,7 +69,7 @@ public class ResourceProviderTracker imp
 
         void providerAdded();
 
-        void providerRemoved(String pid);
+        void providerRemoved(String pid, boolean stateful);
     }
 
     private final Logger logger = LoggerFactory.getLogger(this.getClass());
@@ -128,6 +129,9 @@ public class ResourceProviderTracker imp
     }
 
     public void deactivate() {
+        this.listener = null;
+        this.eventAdmin = null;
+        this.providerReporter = null;
         if ( this.tracker != null ) {
             this.tracker.close();
             this.tracker = null;
@@ -135,7 +139,6 @@ public class ResourceProviderTracker imp
         this.infos.clear();
         this.handlers.clear();
         this.invalidProviders.clear();
-        this.listener = null;
     }
 
     public void setObservationReporterGenerator(final ObservationReporterGenerator generator) {
@@ -182,11 +185,12 @@ public class ResourceProviderTracker imp
                        }
                        events.add(new ProviderEvent(true, info));
                        if ( matchingHandlers.size() > 1 ) {
+                           final ResourceProviderInfo removeInfo = matchingHandlers.get(1).getInfo();
                            if ( cl != null ) {
-                               cl.providerRemoved((String)matchingHandlers.get(1).getInfo().getServiceReference().getProperty(Constants.SERVICE_PID));
+                               cl.providerRemoved((String)removeInfo.getServiceReference().getProperty(Constants.SERVICE_PID), info.getAuthType() != AuthType.no);
                            }
                            this.deactivate(matchingHandlers.get(1));
-                           events.add(new ProviderEvent(false, matchingHandlers.get(1).getInfo()));
+                           events.add(new ProviderEvent(false, removeInfo));
                        }
                    }
                }
@@ -221,7 +225,7 @@ public class ResourceProviderTracker imp
                         doActivateNext = true;
                         final ChangeListener cl = this.listener;
                         if ( cl != null ) {
-                            cl.providerRemoved(pid);
+                            cl.providerRemoved(pid, info.getAuthType() != AuthType.no);
                         }
                         this.deactivate(firstHandler);
                         events.add(new ProviderEvent(false, firstHandler.getInfo()));
@@ -303,13 +307,16 @@ public class ResourceProviderTracker imp
      * @param event
      */
     private void postOSGiEvent(final ProviderEvent event) {
-        final Dictionary<String, Object> eventProps = new Hashtable<String, Object>();
-        eventProps.put(SlingConstants.PROPERTY_PATH, event.path);
-        if (event.pid != null) {
-            eventProps.put(Constants.SERVICE_PID, event.pid);
+        final EventAdmin ea = this.eventAdmin;
+        if ( ea != null ) {
+            final Dictionary<String, Object> eventProps = new Hashtable<String, Object>();
+            eventProps.put(SlingConstants.PROPERTY_PATH, event.path);
+            if (event.pid != null) {
+                eventProps.put(Constants.SERVICE_PID, event.pid);
+            }
+            ea.postEvent(new Event(event.isAdd ? SlingConstants.TOPIC_RESOURCE_PROVIDER_ADDED : SlingConstants.TOPIC_RESOURCE_PROVIDER_REMOVED,
+                    eventProps));
         }
-        eventAdmin.postEvent(new Event(event.isAdd ? SlingConstants.TOPIC_RESOURCE_PROVIDER_ADDED : SlingConstants.TOPIC_RESOURCE_PROVIDER_REMOVED,
-                eventProps));
     }
 
     /**
@@ -318,9 +325,12 @@ public class ResourceProviderTracker imp
      * @param info The resource provider
      */
     private void postResourceProviderChange(final ProviderEvent event) {
-        final ResourceChange change = new ResourceChange(event.isAdd ? ChangeType.PROVIDER_ADDED : ChangeType.PROVIDER_REMOVED,
-                event.path, false, null, null, null);
-        this.providerReporter.reportChanges(Collections.singletonList(change), false);
+        final ObservationReporter or = this.providerReporter;
+        if ( or != null ) {
+            final ResourceChange change = new ResourceChange(event.isAdd ? ChangeType.PROVIDER_ADDED : ChangeType.PROVIDER_REMOVED,
+                    event.path, false, null, null, null);
+            or.reportChanges(Collections.singletonList(change), false);
+        }
     }
 
     public void fill(final RuntimeDTO dto) {
@@ -417,6 +427,9 @@ public class ResourceProviderTracker imp
         if ( events.isEmpty() ) {
             return;
         }
+        if ( this.listener == null && this.providerReporter == null ) {
+            return;
+        }
         final Thread t = new Thread(new Runnable() {
 
             @Override

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java?rev=1735417&r1=1735416&r2=1735417&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java Thu Mar 17 12:56:26 2016
@@ -119,7 +119,7 @@ public class ResourceProviderTrackerTest
             }
 
             @Override
-            public void providerRemoved(String pid) {
+            public void providerRemoved(String pid, boolean stateful) {
                 removedCalled.set(true);
             }
 
@@ -177,7 +177,7 @@ public class ResourceProviderTrackerTest
             }
 
             @Override
-            public void providerRemoved(String pid) {
+            public void providerRemoved(String pid, boolean stateful) {
                 removedCalled.set(true);
             }
 
@@ -291,7 +291,7 @@ public class ResourceProviderTrackerTest
         }
 
         @Override
-        public void providerRemoved(String pid) {
+        public void providerRemoved(String pid, boolean stateful) {
             // TODO Auto-generated method stub
 
         }