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 2018/02/02 06:16:59 UTC
[sling-org-apache-sling-servlets-resolver] branch master updated:
SLING-7456 : FlushCache in SlingServletResolver can throw an NPE
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 <cz...@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.