sling-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] jsedding commented on a change in pull request #6: SLING-8187 - Deadlock in SlingMainServlet after SLING-8051
Date Thu, 20 Dec 2018 14:12:37 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message