jsedding commented on a change in pull request #6: SLING-8187 - Deadlock in
SlingMainServlet after SLING-8051
URL:
https://github.com/apache/sling-org-apache-sling-engine/pull/6#discussion_r243284753
##########
File path: src/main/java/org/apache/sling/engine/impl/SlingMainServlet.java
##########
@@ -424,6 +426,23 @@ protected void activate(final BundleContext bundleContext,
servletConfig.put(Constants.SERVICE_VENDOR, "The Apache Software
Foundation");
this.servletRegistration =
bundleContext.registerService(Servlet.class, this, servletConfig);
+ // note: registration of SlingServletContext as a service is delayed
to the #init() method
+ slingServletContext = new SlingServletContext(bundleContext, this);
+
+ // register render filters already registered after registration with
+ // the HttpService as filter initialization may cause the servlet
+ // context to be required (see SLING-42)
+ filterManager = new ServletFilterManager(bundleContext,
Review comment:
You are right. I think it would be worthwhile to disentangle the various
object instances and services and their respective requirements. I could
imagine the SlingRequestProcessor to be registered as a DS component.
I only fear that there cycles in the object dependency graph, which may be
difficult to resolve. E.g.
- SlingRequestProcessor (via filter manager) requires a reference to the
ServletContext
- ServletContext is only available after SlingMainServlet#init(), thus
requires SlingMainServlet
- SlingMainServlet requires SlingRequestProcessor
Making all registrations asynchronous violates the premise of the Servlet
spect that a servlet is ready to serve requests when
Servlet#init(ServletConfig) returns.
I'm not sure how to address this yet. I have a feeling that it is "wrong" to
register SlingServletContext as a service in the first place. The OSGi HTTP
whiteboard e.g. explicitly requires a per-bundle ServletContext in order to
maintain class-loader isolation (i.e. ServletContext#getClassLoader(), see OSGi
Compendium 140.2).
I'll have to think some more about this. Any ideas or suggestions welcome :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
