sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cziege...@apache.org
Subject [sling-org-apache-sling-servlets-resolver] branch master updated: SLING-7456 : FlushCache in SlingServletResolver can throw an NPE
Date Fri, 02 Feb 2018 06:16:59 GMT
This is an automated email from the ASF dual-hosted git repository.

cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git


The following commit(s) were added to refs/heads/master by this push:
     new 557ad87  SLING-7456 : FlushCache in SlingServletResolver can throw an NPE
557ad87 is described below

commit 557ad87545020a656564441715b4a10c464505b3
Author: Carsten Ziegeler <cziegele@adobe.com>
AuthorDate: Fri Feb 2 07:16:50 2018 +0100

    SLING-7456 : FlushCache in SlingServletResolver can throw an NPE
---
 .../resolver/internal/SlingServletResolver.java    | 55 +++++++++++++++-------
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java
b/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java
index 060bfe6..f29f08c 100644
--- a/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java
+++ b/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java
@@ -96,9 +96,9 @@ import org.osgi.framework.BundleContext;
 import org.osgi.framework.Constants;
 import org.osgi.framework.ServiceReference;
 import org.osgi.framework.ServiceRegistration;
-import org.osgi.service.component.ComponentContext;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
 import org.osgi.service.component.annotations.ReferencePolicy;
@@ -202,7 +202,7 @@ public class SlingServletResolver
     private final List<PendingServlet> pendingServlets = new ArrayList<>();
 
     /** The bundle context. */
-    private BundleContext context;
+    private volatile BundleContext context;
 
     private ServletResourceProviderFactory servletResourceProviderFactory;
 
@@ -215,7 +215,7 @@ public class SlingServletResolver
     private Servlet fallbackErrorServlet;
 
     /** The script resolution cache. */
-    private Map<AbstractResourceCollector, Servlet> cache;
+    private volatile Map<AbstractResourceCollector, Servlet> cache;
 
     /** The cache size. */
     private int cacheSize;
@@ -653,7 +653,9 @@ public class SlingServletResolver
     private Servlet getServletInternal(final AbstractResourceCollector locationUtil,
             final SlingHttpServletRequest request,
             final ResourceResolver resolver) {
-        final Servlet scriptServlet = (this.cache != null ? this.cache.get(locationUtil)
: null);
+        // use local variable to avoid race condition with activate
+        final Map<AbstractResourceCollector, Servlet> localCache = this.cache;
+        final Servlet scriptServlet = (localCache != null ? localCache.get(locationUtil)
: null);
         if (scriptServlet != null) {
             if ( LOGGER.isDebugEnabled() ) {
                 LOGGER.debug("Using cached servlet {}", RequestUtil.getServletName(scriptServlet));
@@ -683,9 +685,9 @@ public class SlingServletResolver
                 final boolean isOptingServlet = candidate instanceof OptingServlet;
                 boolean servletAcceptsRequest = !isOptingServlet || (request != null &&
((OptingServlet) candidate).accepts(request));
                 if (servletAcceptsRequest) {
-                    if (!hasOptingServlet && !isOptingServlet && this.cache
!= null) {
-                        if ( this.cache.size() < this.cacheSize ) {
-                            this.cache.put(locationUtil, candidate);
+                    if (!hasOptingServlet && !isOptingServlet && localCache
!= null) {
+                        if ( localCache.size() < this.cacheSize ) {
+                            localCache.put(locationUtil, candidate);
                         } else if ( this.logCacheSizeWarning ) {
                             this.logCacheSizeWarning = false;
                             LOGGER.warn("Script cache has reached its limit of {}. You might
want to increase the cache size for the servlet resolver.",
@@ -864,7 +866,7 @@ public class SlingServletResolver
         // and finally register as event listener if we need to flush the cache
         if ( this.cache != null ) {
 
-    		final Dictionary<String, Object> props = new Hashtable<>();
+    		    final Dictionary<String, Object> props = new Hashtable<>();
             props.put("event.topics", new String[] {"javax/script/ScriptEngineFactory/*",
                 "org/apache/sling/api/adapter/AdapterFactory/*","org/apache/sling/scripting/core/BindingsValuesProvider/*"
});
             props.put(ResourceChangeListener.PATHS, "/");
@@ -876,7 +878,7 @@ public class SlingServletResolver
         }
 
         this.plugin = new ServletResolverWebConsolePlugin(context);
-        if (this.cacheSize > 0) {
+        if ( this.cache != null ) {
             try {
                 Dictionary<String, String> mbeanProps = new Hashtable<>();
                 mbeanProps.put("jmx.objectname", "org.apache.sling:type=servletResolver,service=SlingServletResolverCache");
@@ -891,17 +893,22 @@ public class SlingServletResolver
     }
 
     private void updateScriptEngineExtensions() {
-        List<String> scriptEnginesExtensions = new ArrayList<>();
-        for (ScriptEngineFactory factory : scriptEngineManager.getEngineFactories()) {
-            scriptEnginesExtensions.addAll(factory.getExtensions());
+        final ScriptEngineManager localScriptEngineManager = scriptEngineManager;
+        // use local variable to avoid racing with deactivate
+        if ( localScriptEngineManager != null ) {
+            final List<String> scriptEnginesExtensions = new ArrayList<>();
+            for (ScriptEngineFactory factory : localScriptEngineManager.getEngineFactories())
{
+                scriptEnginesExtensions.addAll(factory.getExtensions());
+            }
+            this.scriptEnginesExtensions = Collections.unmodifiableList(scriptEnginesExtensions);
         }
-        this.scriptEnginesExtensions = Collections.unmodifiableList(scriptEnginesExtensions);
     }
 
     /**
      * Deactivate this component.
      */
-    protected void deactivate(final ComponentContext context) {
+    @Deactivate
+    protected void deactivate() {
         // stop registering of servlets immediately
         this.context = null;
 
@@ -1114,13 +1121,21 @@ public class SlingServletResolver
      */
     @Override
     public void handleEvent(final Event event) {
+        // return immediately if already deactivated
+        if ( this.context == null ) {
+            return;
+        }
         flushCache();
         updateScriptEngineExtensions();
     }
 
     private void flushCache() {
-        this.cache.clear();
-        this.logCacheSizeWarning = true;
+        // use local variable to avoid racing with deactivate
+        final Map<AbstractResourceCollector, Servlet> localCache = this.cache;
+        if ( localCache != null ) {
+            localCache.clear();
+            this.logCacheSizeWarning = true;
+        }
     }
 
     /** The list of property names checked by {@link #getName(ServiceReference)} */
@@ -1431,7 +1446,9 @@ public class SlingServletResolver
 
         @Override
         public int getCacheSize() {
-            return cache != null ? cache.size() : 0;
+            // use local variable to avoid racing with deactivate
+            final Map<AbstractResourceCollector, Servlet> localCache = cache;
+            return localCache != null ? localCache.size() : 0;
         }
 
         @Override
@@ -1448,6 +1465,10 @@ public class SlingServletResolver
 
     @Override
 	public void onChange(final List<ResourceChange> changes) {
+        // return immediately if already deactivated
+        if ( context == null ) {
+            return;
+        }
         boolean flushCache = false;
         for(final ResourceChange change : changes){
             // if the path of the event is a sub path of a search path

-- 
To stop receiving notification emails like this one, please contact
cziegeler@apache.org.

Mime
View raw message