Return-Path: Delivered-To: apmail-sling-commits-archive@www.apache.org Received: (qmail 46841 invoked from network); 5 Jul 2010 13:18:52 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 5 Jul 2010 13:18:52 -0000 Received: (qmail 60408 invoked by uid 500); 5 Jul 2010 13:18:52 -0000 Delivered-To: apmail-sling-commits-archive@sling.apache.org Received: (qmail 60352 invoked by uid 500); 5 Jul 2010 13:18:50 -0000 Mailing-List: contact commits-help@sling.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@sling.apache.org Delivered-To: mailing list commits@sling.apache.org Received: (qmail 60340 invoked by uid 99); 5 Jul 2010 13:18:49 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 05 Jul 2010 13:18:49 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 05 Jul 2010 13:18:46 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 0CF1323889E1; Mon, 5 Jul 2010 13:17:53 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r960592 - /sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java Date: Mon, 05 Jul 2010 13:17:53 -0000 To: commits@sling.apache.org From: cziegeler@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20100705131753.0CF1323889E1@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: cziegeler Date: Mon Jul 5 13:17:52 2010 New Revision: 960592 URL: http://svn.apache.org/viewvc?rev=960592&view=rev Log: SLING-1580 : SlingServletResolver registers OSGi services from synchronized blocks Modified: sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java Modified: sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java?rev=960592&r1=960591&r2=960592&view=diff ============================================================================== --- sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java (original) +++ sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/SlingServletResolver.java Mon Jul 5 13:17:52 2010 @@ -166,9 +166,9 @@ public class SlingServletResolver implem private ResourceResolver scriptResolver; - private Map servletsByReference = new HashMap(); + private final Map servletsByReference = new HashMap(); - private List pendingServlets = new ArrayList(); + private final List pendingServlets = new ArrayList(); /** The component context. */ private ComponentContext context; @@ -773,10 +773,10 @@ public class SlingServletResolver implem return null; } - private Map createAuthenticationInfo() { + private Map createAuthenticationInfo(final Dictionary props) { final Map authInfo = new HashMap(); // if a script user is configured we use this user to read the scripts - final String scriptUser = OsgiUtil.toString(context.getProperties().get(PROP_SCRIPT_USER), null); + final String scriptUser = OsgiUtil.toString(props.get(PROP_SCRIPT_USER), null); if (scriptUser != null && scriptUser.length() > 0) { authInfo.put(ResourceResolverFactory.SUDO_USER_ID, scriptUser); } @@ -788,7 +788,8 @@ public class SlingServletResolver implem /** * Activate this component. */ - protected void activate(ComponentContext context) throws LoginException { + @SuppressWarnings("unchecked") + protected void activate(final ComponentContext context) throws LoginException { // from configuration if available final Dictionary properties = context.getProperties(); Object servletRoot = properties.get(PROP_SERVLET_ROOT); @@ -796,30 +797,31 @@ public class SlingServletResolver implem servletRoot = DEFAULT_SERVLET_ROOT; } - final Collection refs; - synchronized (this) { + // workspace handling and resource resolver creation + this.useDefaultWorkspace = OsgiUtil.toBoolean(properties.get(PROP_USE_DEFAULT_WORKSPACE), DEFAULT_USE_DEFAULT_WORKSPACE); + this.useRequestWorkspace = OsgiUtil.toBoolean(properties.get(PROP_USE_REQUEST_WORKSPACE), DEFAULT_USE_REQUEST_WORKSPACE); + + String defaultWorkspaceProp = (String) properties.get(PROP_DEFAULT_SCRIPT_WORKSPACE); + if ( defaultWorkspaceProp != null && defaultWorkspaceProp.trim().length() == 0 ) { + defaultWorkspaceProp = null; + } + this.defaultWorkspaceName = defaultWorkspaceProp; - refs = pendingServlets; - pendingServlets = new ArrayList(); - // register servlets immediately from now on - this.context = context; + final Collection refs; + synchronized (this.pendingServlets) { - // workspace handling and resource resolver creation - this.useDefaultWorkspace = OsgiUtil.toBoolean(properties.get(PROP_USE_DEFAULT_WORKSPACE), DEFAULT_USE_DEFAULT_WORKSPACE); - this.useRequestWorkspace = OsgiUtil.toBoolean(properties.get(PROP_USE_REQUEST_WORKSPACE), DEFAULT_USE_REQUEST_WORKSPACE); - - String defaultWorkspaceProp = (String) properties.get(PROP_DEFAULT_SCRIPT_WORKSPACE); - if ( defaultWorkspaceProp != null && defaultWorkspaceProp.trim().length() == 0 ) { - defaultWorkspaceProp = null; - } - this.defaultWorkspaceName = defaultWorkspaceProp; + refs = new ArrayList(pendingServlets); + pendingServlets.clear(); this.scriptResolver = - resourceResolverFactory.getAdministrativeResourceResolver(this.createAuthenticationInfo()); + resourceResolverFactory.getAdministrativeResourceResolver(this.createAuthenticationInfo(context.getProperties())); servletResourceProviderFactory = new ServletResourceProviderFactory(servletRoot, this.scriptResolver.getSearchPath()); + + // register servlets immediately from now on + this.context = context; } createAllServlets(refs); @@ -861,7 +863,10 @@ public class SlingServletResolver implem /** * Deactivate this component. */ - protected void deactivate(ComponentContext context) { + protected void deactivate(final ComponentContext context) { + // stop registering of servlets immediately + this.context = null; + // unregister event handler if (this.eventHandlerReg != null) { this.eventHandlerReg.unregister(); @@ -871,12 +876,17 @@ public class SlingServletResolver implem // Copy the list of servlets first, to minimize the need for // synchronization final Collection refs; - synchronized (this) { + synchronized (this.servletsByReference) { refs = new ArrayList(servletsByReference.keySet()); } // destroy all servlets destroyAllServlets(refs); + // sanity check: clear array (it should be empty now anyway) + synchronized ( this.servletsByReference ) { + this.servletsByReference.clear(); + } + // destroy the fallback error handler servlet if (fallbackErrorServlet != null) { try { @@ -893,33 +903,41 @@ public class SlingServletResolver implem this.scriptResolver = null; } - this.context = null; this.cache = null; this.servletResourceProviderFactory = null; } - protected synchronized void bindServlet(ServiceReference reference) { + protected void bindServlet(ServiceReference reference) { + boolean directCreate = true; if (context == null) { - pendingServlets.add(reference); - } else { - createServlet(servletContext, reference); + synchronized ( pendingServlets ) { + if (context == null) { + pendingServlets.add(reference); + directCreate = false; + } + } + } + if ( directCreate ) { + createServlet(reference); } } - protected synchronized void unbindServlet(ServiceReference reference) { - pendingServlets.remove(reference); + protected void unbindServlet(ServiceReference reference) { + synchronized ( pendingServlets ) { + pendingServlets.remove(reference); + } destroyServlet(reference); } // ---------- Servlet Management ------------------------------------------- - private void createAllServlets(Collection pendingServlets) { - for (ServiceReference serviceReference : pendingServlets) { - createServlet(servletContext, serviceReference); + private void createAllServlets(final Collection pendingServlets) { + for (final ServiceReference serviceReference : pendingServlets) { + createServlet(serviceReference); } } - private boolean createServlet(ServletContext servletContext, ServiceReference reference) { + private boolean createServlet(final ServiceReference reference) { // check for a name, this is required final String name = getName(reference); @@ -967,11 +985,13 @@ public class SlingServletResolver implem params.put(Constants.SERVICE_DESCRIPTION, "ServletResourceProvider for Servlets at " + Arrays.asList(provider.getServletPaths())); - ServiceRegistration reg = context.getBundleContext().registerService(ResourceProvider.SERVICE_NAME, provider, - params); + final ServiceRegistration reg = context.getBundleContext() + .registerService(ResourceProvider.SERVICE_NAME, provider, params); log.info("Registered {}", provider.toString()); - servletsByReference.put(reference, reg); + synchronized (this.servletsByReference) { + servletsByReference.put(reference, new ServletReg(servlet, reg)); + } return true; } @@ -983,23 +1003,20 @@ public class SlingServletResolver implem } private void destroyServlet(ServiceReference reference) { - ServiceRegistration registration = servletsByReference.remove(reference); + ServletReg registration; + synchronized (this.servletsByReference) { + registration = servletsByReference.remove(reference); + } if (registration != null) { - registration.unregister(); + registration.registration.unregister(); + final String name = RequestUtil.getServletName(registration.servlet); + log.debug("unbindServlet: Servlet {} removed", name); - Servlet servlet = (Servlet) context.locateService(REF_SERVLET, reference); - if (servlet == null) { - log.error("destroyServlet: Servlet not found for reference {}", reference.toString()); - } else { - String name = RequestUtil.getServletName(servlet); - log.debug("unbindServlet: Servlet {} removed", name); - - try { - servlet.destroy(); - } catch (Throwable t) { - log.error("unbindServlet: Unexpected problem destroying servlet " + name, t); - } + try { + registration.servlet.destroy(); + } catch (Throwable t) { + log.error("unbindServlet: Unexpected problem destroying servlet " + name, t); } } } @@ -1069,4 +1086,14 @@ public class SlingServletResolver implem private boolean isPathAllowed(final String path) { return AbstractResourceCollector.isPathAllowed(path, this.executionPaths); } + + private static final class ServletReg { + public final Servlet servlet; + public final ServiceRegistration registration; + + public ServletReg(final Servlet s, final ServiceRegistration sr) { + this.servlet = s; + this.registration = sr; + } + } }