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 2020/01/13 08:20:19 UTC
[sling-org-apache-sling-engine] branch master updated: SLING-8991 :
Avoid recreation of main servlet on config change SLING-8992 : Sling main
servlet name is not set after move to http whiteboard
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-engine.git
The following commit(s) were added to refs/heads/master by this push:
new dd431d8 SLING-8991 : Avoid recreation of main servlet on config change SLING-8992 : Sling main servlet name is not set after move to http whiteboard
dd431d8 is described below
commit dd431d8b0dc782ff587d8540a7dafef1fd766376
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Mon Jan 13 09:20:06 2020 +0100
SLING-8991 : Avoid recreation of main servlet on config change
SLING-8992 : Sling main servlet name is not set after move to http whiteboard
---
.../sling/engine/impl/DefaultErrorHandler.java | 22 +--
.../apache/sling/engine/impl/SlingMainServlet.java | 171 ++++++++++++---------
.../engine/impl/SlingRequestProcessorImpl.java | 7 +-
.../sling/engine/impl/request/RequestData.java | 10 +-
.../impl/request/RequestHistoryConsolePlugin.java | 17 +-
5 files changed, 133 insertions(+), 94 deletions(-)
diff --git a/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java b/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java
index 5a98c57..9daf919 100644
--- a/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java
+++ b/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java
@@ -45,8 +45,8 @@ public class DefaultErrorHandler implements ErrorHandler {
/** default log */
private final Logger log = LoggerFactory.getLogger(getClass());
- private String serverInfo = SlingMainServlet.PRODUCT_NAME;
-
+ private volatile String serverInfo = SlingMainServlet.PRODUCT_NAME;
+
/** Use this if not null, and if that fails output a report about that failure */
private ErrorHandler delegate;
@@ -55,15 +55,15 @@ public class DefaultErrorHandler implements ErrorHandler {
? serverInfo
: SlingMainServlet.PRODUCT_NAME;
}
-
+
public void setDelegate(ErrorHandler eh) {
delegate = eh;
}
-
+
public ErrorHandler getDelegate() {
return delegate;
}
-
+
private void delegateFailed(int originalStatus, String originalMessage, Throwable t, HttpServletRequest request, HttpServletResponse response) throws IOException {
// don't include Throwable in the response, gives too much information
final String m = "Error handler failed:" + t.getClass().getName();
@@ -84,13 +84,14 @@ public class DefaultErrorHandler implements ErrorHandler {
* This method logs error and does not write back and response data if the
* response has already been committed.
*/
+ @Override
public void handleError(final int status,
String message,
final SlingHttpServletRequest request,
final SlingHttpServletResponse response)
throws IOException {
-
- // If we have a delegate let it handle the error
+
+ // If we have a delegate let it handle the error
if(delegate != null) {
try {
delegate.handleError(status, message, request, response);
@@ -101,7 +102,7 @@ public class DefaultErrorHandler implements ErrorHandler {
}
return;
}
-
+
if (message == null) {
message = "HTTP ERROR:" + String.valueOf(status);
} else {
@@ -122,12 +123,13 @@ public class DefaultErrorHandler implements ErrorHandler {
* This method logs error and does not write back and response data if the
* response has already been committed.
*/
+ @Override
public void handleError(final Throwable throwable,
final SlingHttpServletRequest request,
final SlingHttpServletResponse response)
throws IOException {
final int status = HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
- // If we have a delegate let it handle the error
+ // If we have a delegate let it handle the error
if(delegate != null) {
try {
delegate.handleError(throwable, request, response);
@@ -138,7 +140,7 @@ public class DefaultErrorHandler implements ErrorHandler {
}
return;
}
-
+
sendError(status,
throwable.getMessage(), throwable, request, response);
}
diff --git a/src/main/java/org/apache/sling/engine/impl/SlingMainServlet.java b/src/main/java/org/apache/sling/engine/impl/SlingMainServlet.java
index e9cd272..fb3426d 100644
--- a/src/main/java/org/apache/sling/engine/impl/SlingMainServlet.java
+++ b/src/main/java/org/apache/sling/engine/impl/SlingMainServlet.java
@@ -20,12 +20,12 @@ package org.apache.sling.engine.impl;
import java.util.ArrayList;
import java.util.Dictionary;
-import java.util.Enumeration;
import java.util.Hashtable;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
import javax.servlet.GenericServlet;
@@ -58,6 +58,7 @@ import org.osgi.framework.Version;
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.Modified;
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;
@@ -128,6 +129,9 @@ public class SlingMainServlet extends GenericServlet {
description = "Provides mappings for additional response headers "
+ "Each entry is of the form 'bundleId [ \":\" responseHeaderName ] \"=\" responseHeaderValue'")
String[] sling_additional_response_headers() default {"X-Content-Type-Options=nosniff", "X-Frame-Options=SAMEORIGIN"};
+
+ @AttributeDefinition(name = "Servlet Name", description = "Optional name for the Sling main servlet registered by this component")
+ String servlet_name();
}
private static final String DEPRECATED_ENCODING_PROPERTY = "sling.default.parameter.encoding";
@@ -157,7 +161,7 @@ public class SlingMainServlet extends GenericServlet {
*/
static String PRODUCT_NAME = "ApacheSling";
- private SlingServletContext slingServletContext;
+ private volatile SlingServletContext slingServletContext;
/**
* The product information part of the {@link #serverInfo} returns from the
@@ -166,7 +170,7 @@ public class SlingMainServlet extends GenericServlet {
* of the Sling Engine bundle while this component is being
* {@link #activate(BundleContext, Map, Config)} activated}.
*/
- private String productInfo = PRODUCT_NAME;
+ private volatile String productInfo = PRODUCT_NAME;
/**
* The server information to report in the {@link #getServerInfo()} method.
@@ -177,33 +181,35 @@ public class SlingMainServlet extends GenericServlet {
* Finally during servlet initialization the product information from the
* servlet container's server info is added to the comment section.
*/
- private String serverInfo = PRODUCT_NAME;
+ private volatile String serverInfo = PRODUCT_NAME;
- private RequestListenerManager requestListenerManager;
+ private volatile RequestListenerManager requestListenerManager;
- private boolean allowTrace;
+ private volatile boolean allowTrace;
- private Object printerRegistration;
+ private volatile Object printerRegistration;
// new properties
- private SlingHttpContext slingHttpContext = new SlingHttpContext();
+ private final SlingHttpContext slingHttpContext = new SlingHttpContext();
- private ServletFilterManager filterManager;
+ private volatile ServletFilterManager filterManager;
private final SlingRequestProcessorImpl requestProcessor = new SlingRequestProcessorImpl();
- private ServiceRegistration<SlingRequestProcessor> requestProcessorRegistration;
+ private volatile ServiceRegistration<SlingRequestProcessor> requestProcessorRegistration;
+
+ private volatile ServiceRegistration<RequestProcessorMBean> requestProcessorMBeanRegistration;
- private ServiceRegistration<RequestProcessorMBean> requestProcessorMBeanRegistration;
+ private volatile ServiceRegistration<ServletContextHelper> contextRegistration;
- private ServiceRegistration<ServletContextHelper> contextRegistration;
+ private volatile ServiceRegistration<Servlet> servletRegistration;
- private ServiceRegistration<Servlet> servletRegistration;
+ private volatile String configuredServerInfo;
- private String configuredServerInfo;
+ private final CountDownLatch asyncActivation = new CountDownLatch(1);
- private CountDownLatch asyncActivation = new CountDownLatch(1);
+ private final AtomicBoolean isModification = new AtomicBoolean();
// ---------- Servlet API -------------------------------------------------
@@ -300,7 +306,7 @@ public class SlingMainServlet extends GenericServlet {
* @param bundleContext Provides access to the "Bundle-Version" manifest
* header of the containing bundle.
*/
- private void setProductInfo(final BundleContext bundleContext) {
+ private void setProductInfo() {
final Dictionary<?, ?> props = bundleContext.getBundle().getHeaders();
final Version bundleVersion = Version.parseVersion((String) props.get(Constants.BUNDLE_VERSION));
final String productVersion = bundleVersion.getMajor() + "."
@@ -363,20 +369,32 @@ public class SlingMainServlet extends GenericServlet {
System.getProperty("java.version"), System.getProperty("os.name"),
System.getProperty("os.version"), System.getProperty("os.arch"));
}
- if (this.requestProcessor != null) {
- this.requestProcessor.setServerInfo(serverInfo);
- }
+ this.requestProcessor.setServerInfo(serverInfo);
}
// ---------- Property Setter for SCR --------------------------------------
- @Activate
- protected void activate(final BundleContext bundleContext,
- final Map<String, Object> componentConfig,
+ @Modified
+ protected void modified(final Map<String, Object> componentConfig,
final Config config) {
+ this.isModification.set(true);
- this.bundleContext = bundleContext;
+ setup(componentConfig, config);
+ }
+
+ private Dictionary<String, Object> getServletContextRegistrationProps(final String servletName) {
+ final Dictionary<String, Object> servletConfig = new Hashtable<>();
+ servletConfig.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_SELECT,
+ "(" + HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME + "=" + SERVLET_CONTEXT_NAME + ")");
+ servletConfig.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, SLING_ROOT);
+ servletConfig.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_NAME, servletName);
+ servletConfig.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Engine Main Servlet");
+ servletConfig.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
+
+ return servletConfig;
+ }
+ protected void setup(final Map<String, Object> componentConfig, final Config config) {
final String[] props = config.sling_additional_response_headers();
if ( props != null ) {
final ArrayList<StaticResponseHeader> mappings = new ArrayList<>(props.length);
@@ -392,29 +410,25 @@ public class SlingMainServlet extends GenericServlet {
}
RequestData.setAdditionalResponseHeaders(mappings);
}
- configuredServerInfo = config.sling_serverinfo();
- // setup server info
- setProductInfo(bundleContext);
-
- // prepare the servlet configuration from the component config
- final Hashtable<String, Object> configuration = new Hashtable<>(
- componentConfig);
-
- // ensure the servlet name
- if (!(configuration.get("servlet-name") instanceof String)) {
- configuration.put("servlet-name", this.productInfo);
+ if (config.sling_serverinfo() != null && !config.sling_serverinfo().isEmpty()) {
+ this.configuredServerInfo = config.sling_serverinfo();
+ } else {
+ this.configuredServerInfo = null;
}
+ // setup server info
+ setProductInfo();
+
// configure method filter
- allowTrace = config.sling_trace_allow();
+ this.allowTrace = config.sling_trace_allow();
// configure the request limits
RequestData.setMaxIncludeCounter(config.sling_max_inclusions());
RequestData.setMaxCallCounter(config.sling_max_calls());
RequestData.setSlingMainServlet(this);
- // Warn about the obsolete parameter encoding configuration
+ // Warn about the obsolete parameter encoding configuration (SLING-5370)
if (componentConfig.get(DEPRECATED_ENCODING_PROPERTY) != null) {
log.warn("Please configure the default request parameter encoding using "
+ "the 'org.apache.sling.engine.parameters' configuration PID; the property "
@@ -423,23 +437,32 @@ public class SlingMainServlet extends GenericServlet {
+ " is obsolete and ignored");
}
- // register the servlet context
- final Dictionary<String, String> contextProperties = new Hashtable<>();
- contextProperties.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME, SERVLET_CONTEXT_NAME);
- contextProperties.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_PATH, SLING_ROOT);
- contextProperties.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Engine Servlet Context Helper");
- contextProperties.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
+ if (this.contextRegistration == null) {
+ // register the servlet context
+ final Dictionary<String, String> contextProperties = new Hashtable<>();
+ contextProperties.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME, SERVLET_CONTEXT_NAME);
+ contextProperties.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_PATH, SLING_ROOT);
+ contextProperties.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Engine Servlet Context Helper");
+ contextProperties.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
- this.contextRegistration = bundleContext.registerService(ServletContextHelper.class, this.slingHttpContext, contextProperties);
+ this.contextRegistration = bundleContext.registerService(ServletContextHelper.class, this.slingHttpContext,
+ contextProperties);
+ }
- // register the servlet
- final Dictionary<String, String> servletConfig = toStringConfig(configuration);
- servletConfig.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_SELECT,
- "(" + HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME + "=" + SERVLET_CONTEXT_NAME + ")");
- servletConfig.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, SLING_ROOT);
- servletConfig.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Engine Main Servlet");
- servletConfig.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
- this.servletRegistration = bundleContext.registerService(Servlet.class, this, servletConfig);
+ String servletName = config.servlet_name();
+ if (servletName == null || servletName.isEmpty()) {
+ servletName = this.productInfo;
+ }
+ if (this.servletRegistration == null) {
+ this.servletRegistration = bundleContext.registerService(Servlet.class, this,
+ getServletContextRegistrationProps(servletName));
+ } else {
+ // check if the servlet name has changed and update properties
+ if (!servletName.equals(this.servletRegistration.getReference()
+ .getProperty(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_NAME))) {
+ this.servletRegistration.setProperties(getServletContextRegistrationProps(servletName));
+ }
+ }
// setup the request info recorder
try {
@@ -459,11 +482,29 @@ public class SlingMainServlet extends GenericServlet {
}
}
+ @Activate
+ protected void activate(final BundleContext bundleContext, final Map<String, Object> componentConfig,
+ final Config config) {
+
+ this.bundleContext = bundleContext;
+ this.isModification.set(false);
+ this.setup(componentConfig, config);
+ }
+
@Override
- public void init() {
+ public void init() throws ServletException {
setServerInfo();
log.info("{} ready to serve requests", this.getServerInfo());
- asyncSlingServletContextRegistration();
+ if (slingServletContext == null) {
+ asyncSlingServletContextRegistration();
+ }
+ }
+
+ @Override
+ public void destroy() {
+ if (!this.isModification.compareAndSet(true, false)) {
+ unregisterSlingServletContext();
+ }
}
// registration needs to be async. if it is done synchronously
@@ -524,7 +565,6 @@ public class SlingMainServlet extends GenericServlet {
log.warn("Async activation did not complete within 30 seconds of 'deactivate' " +
"being called. There is a risk that objects are not properly destroyed.");
}
- unregisterSlingServletContext();
// unregister request recorder plugin
try {
@@ -534,6 +574,13 @@ public class SlingMainServlet extends GenericServlet {
"Problem unregistering web console request recorder plugin.", t);
}
+ // second unregister the servlet context *before* unregistering
+ // and destroying the the sling main servlet
+ if (this.contextRegistration != null) {
+ this.contextRegistration.unregister();
+ this.contextRegistration = null;
+ }
+
// third unregister and destroy the sling main servlet
// unregister servlet
if ( this.servletRegistration != null ) {
@@ -578,14 +625,9 @@ public class SlingMainServlet extends GenericServlet {
if (filterManager != null) {
requestProcessor.setFilterManager(null);
filterManager.close();
+ filterManager = null;
}
- // second unregister the servlet context *before* unregistering
- // and destroying the the sling main servlet
- if ( this.contextRegistration != null ) {
- this.contextRegistration.unregister();
- this.contextRegistration = null;
- }
if (slingServletContext != null) {
slingServletContext.dispose();
slingServletContext = null;
@@ -630,15 +672,6 @@ public class SlingMainServlet extends GenericServlet {
slingHttpContext.unsetAuthenticationSupport(authenticationSupport);
}
- private Dictionary<String, String> toStringConfig(Dictionary<?, ?> config) {
- Dictionary<String, String> stringConfig = new Hashtable<>();
- for (Enumeration<?> ke = config.keys(); ke.hasMoreElements();) {
- Object key = ke.nextElement();
- stringConfig.put(key.toString(), String.valueOf(config.get(key)));
- }
- return stringConfig;
- }
-
// ---------- HttpContext interface ----------------------------------------
public String getMimeType(String name) {
diff --git a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
index 710a7eb..90af1ee 100644
--- a/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
+++ b/src/main/java/org/apache/sling/engine/impl/SlingRequestProcessorImpl.java
@@ -70,11 +70,11 @@ public class SlingRequestProcessorImpl implements SlingRequestProcessor {
private final DefaultErrorHandler errorHandler = new DefaultErrorHandler();
- private ServletResolver servletResolver;
+ private volatile ServletResolver servletResolver;
- private ServletFilterManager filterManager;
+ private volatile ServletFilterManager filterManager;
- private RequestProcessorMBeanImpl mbean;
+ private volatile RequestProcessorMBeanImpl mbean;
// ---------- helper setters
@@ -237,6 +237,7 @@ public class SlingRequestProcessorImpl implements SlingRequestProcessor {
/**
* @see org.apache.sling.engine.SlingRequestProcessor#processRequest(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse, org.apache.sling.api.resource.ResourceResolver)
*/
+ @Override
public void processRequest(final HttpServletRequest servletRequest,
final HttpServletResponse servletResponse,
final ResourceResolver resourceResolver) throws IOException {
diff --git a/src/main/java/org/apache/sling/engine/impl/request/RequestData.java b/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
index 9565345..379eb83 100644
--- a/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
+++ b/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
@@ -102,7 +102,7 @@ public class RequestData {
* number of entries in the {@link #contentDataStack} when the
* {@link #pushContent(Resource, RequestPathInfo)} method is called.
*/
- private static int maxInclusionCounter = DEFAULT_MAX_INCLUSION_COUNTER;
+ private static volatile int maxInclusionCounter = DEFAULT_MAX_INCLUSION_COUNTER;
/**
* The maximum number of scripts which may be included through the
@@ -110,18 +110,18 @@ public class RequestData {
* method (default {@link #DEFAULT_MAX_CALL_COUNTER}). This number should
* not be too small to prevent request aborts.
*/
- private static int maxCallCounter = DEFAULT_MAX_CALL_COUNTER;
+ private static volatile int maxCallCounter = DEFAULT_MAX_CALL_COUNTER;
/**
* The name of the request attribute to override the max call number (-1 for infinite or integer value).
*/
private static String REQUEST_MAX_CALL_OVERRIDE = "sling.max.calls";
- private static SlingMainServlet SLING_MAIN_SERVLET;
+ private static volatile SlingMainServlet SLING_MAIN_SERVLET;
- private static SlingHttpServletRequestFactory REQUEST_FACTORY;
+ private static volatile SlingHttpServletRequestFactory REQUEST_FACTORY;
- private static ArrayList<StaticResponseHeader> ADDITIONAL_RESPONSE_HEADERS;
+ private static volatile ArrayList<StaticResponseHeader> ADDITIONAL_RESPONSE_HEADERS;
/** The SlingMainServlet used for request dispatching and other stuff */
private final SlingRequestProcessorImpl slingRequestProcessor;
diff --git a/src/main/java/org/apache/sling/engine/impl/request/RequestHistoryConsolePlugin.java b/src/main/java/org/apache/sling/engine/impl/request/RequestHistoryConsolePlugin.java
index f48a276..c15f5ea 100644
--- a/src/main/java/org/apache/sling/engine/impl/request/RequestHistoryConsolePlugin.java
+++ b/src/main/java/org/apache/sling/engine/impl/request/RequestHistoryConsolePlugin.java
@@ -29,6 +29,7 @@ import java.util.List;
import java.util.concurrent.atomic.AtomicLong;
import java.util.regex.Pattern;
+import javax.servlet.Servlet;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -58,7 +59,7 @@ public class RequestHistoryConsolePlugin {
private static Plugin instance;
- private static ServiceRegistration serviceRegistration;
+ private static ServiceRegistration<Servlet> serviceRegistration;
public static final int STORED_REQUESTS_COUNT = 20;
@@ -73,7 +74,8 @@ public class RequestHistoryConsolePlugin {
public static void initPlugin(BundleContext context, int maxRequests, List<Pattern> storePatterns) {
if (instance == null) {
- Plugin tmp = new Plugin(maxRequests, storePatterns);
+ final Plugin tmp = new Plugin();
+ instance.update(maxRequests, storePatterns);
final Dictionary<String, Object> props = new Hashtable<String, Object>();
props.put(Constants.SERVICE_DESCRIPTION,
"Web Console Plugin to display information about recent Sling requests");
@@ -84,9 +86,10 @@ public class RequestHistoryConsolePlugin {
props.put("felix.webconsole.title", "Recent requests");
props.put("felix.webconsole.category", "Sling");
- serviceRegistration = context.registerService(
- "javax.servlet.Servlet", tmp, props);
+ serviceRegistration = context.registerService(Servlet.class, tmp, props);
instance = tmp;
+ } else {
+ instance.update(maxRequests, storePatterns);
}
}
@@ -105,11 +108,11 @@ public class RequestHistoryConsolePlugin {
public static final class Plugin extends HttpServlet {
- private final RequestInfoMap requests;
+ private volatile RequestInfoMap requests;
- private final List<Pattern> storePatterns;
+ private volatile List<Pattern> storePatterns;
- Plugin(int maxRequests, List<Pattern> storePatterns) {
+ public void update(int maxRequests, List<Pattern> storePatterns) {
this.requests = (maxRequests > 0)
? new RequestInfoMap(maxRequests)
: null;