[jboss-dev-forums] [jbossws-commits] JBossWS SVN: r10712 - spi/trunk/src/main/java/org/jboss/wsf/spi/util.
Richard Opalka
ropalka at redhat.com
Mon Sep 21 01:43:34 EDT 2009
Hi Alessio,
I reviewed this commit and I don't like it.
It introduces another memory leak :(
Please consider the following fix in ResourceCachingClassLoader:
* use WeakHashMap instead of ConcurrentHashMap
* use synchronized keyword in public methods (or other concurrency
mechanism to synchronize hash map access)
Thanks,
Richard
On 09/18/2009 10:56 AM, jbossws-commits at lists.jboss.org wrote:
> Author: alessio.soldano at jboss.com
> Date: 2009-09-18 04:56:42 -0400 (Fri, 18 Sep 2009)
> New Revision: 10712
>
> Added:
> spi/trunk/src/main/java/org/jboss/wsf/spi/util/ResourceCachingClassLoader.java
> Modified:
> spi/trunk/src/main/java/org/jboss/wsf/spi/util/SecurityActions.java
> spi/trunk/src/main/java/org/jboss/wsf/spi/util/ServiceLoader.java
> Log:
> [JBWS-2763] Cache resource name lookup through Services API (prevent many useless accesses to filesystem)
>
>
> Added: spi/trunk/src/main/java/org/jboss/wsf/spi/util/ResourceCachingClassLoader.java
> ===================================================================
> --- spi/trunk/src/main/java/org/jboss/wsf/spi/util/ResourceCachingClassLoader.java (rev 0)
> +++ spi/trunk/src/main/java/org/jboss/wsf/spi/util/ResourceCachingClassLoader.java 2009-09-18 08:56:42 UTC (rev 10712)
> @@ -0,0 +1,59 @@
> +/*
> + * JBoss, Home of Professional Open Source.
> + * Copyright 2009, Red Hat Middleware LLC, and individual contributors
> + * as indicated by the @author tags. See the copyright.txt file in the
> + * distribution for a full listing of individual contributors.
> + *
> + * This is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as
> + * published by the Free Software Foundation; either version 2.1 of
> + * the License, or (at your option) any later version.
> + *
> + * This software is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this software; if not, write to the Free
> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
> + */
> +package org.jboss.wsf.spi.util;
> +
> +import java.util.concurrent.ConcurrentHashMap;
> +import java.util.concurrent.ConcurrentMap;
> +
> +/**
> + * A ClassLoader with caches for resource lookup through services (META-INF/services)
> + *
> + * @author alessio.soldano at jboss.com
> + * @since 18-Sep-2009
> + *
> + */
> +public class ResourceCachingClassLoader extends ClassLoader
> +{
> + private ConcurrentMap<String, String> servicesMap = new ConcurrentHashMap<String, String>();
> +
> + public ResourceCachingClassLoader(ClassLoader parent)
> + {
> + super(parent);
> + //clear maps to same memory, as this constructor is also called when this classloader becomes a father (hence the cache is not useful anymore)
> + servicesMap.clear();
> + }
> +
> + public boolean hasResourceNameFromServices(String key)
> + {
> + return servicesMap.containsKey(key);
> + }
> +
> + public String getResourceNameFromServices(String key)
> + {
> + return servicesMap.get(key);
> + }
> +
> + public void setResourceNameFromServices(String key, String value)
> + {
> + servicesMap.put(key, value);
> + }
> +}
>
>
> Property changes on: spi/trunk/src/main/java/org/jboss/wsf/spi/util/ResourceCachingClassLoader.java
> ___________________________________________________________________
> Name: svn:keywords
> + Id Revision
> Name: svn:eol-style
> + LF
>
> Modified: spi/trunk/src/main/java/org/jboss/wsf/spi/util/SecurityActions.java
> ===================================================================
> --- spi/trunk/src/main/java/org/jboss/wsf/spi/util/SecurityActions.java 2009-09-17 15:30:33 UTC (rev 10711)
> +++ spi/trunk/src/main/java/org/jboss/wsf/spi/util/SecurityActions.java 2009-09-18 08:56:42 UTC (rev 10712)
> @@ -60,6 +60,29 @@
> }
>
> /**
> + * Set context classloader.
> + *
> + */
> + static void setContextClassLoader(final ClassLoader cl)
> + {
> + SecurityManager sm = System.getSecurityManager();
> + if (sm == null)
> + {
> + Thread.currentThread().setContextClassLoader(cl);
> + }
> + else
> + {
> + AccessController.doPrivileged(new PrivilegedAction<Object>() {
> + public Object run()
> + {
> + Thread.currentThread().setContextClassLoader(cl);
> + return null;
> + }
> + });
> + }
> + }
> +
> + /**
> * Get resource as stream
> *
> * @param cl
>
> Modified: spi/trunk/src/main/java/org/jboss/wsf/spi/util/ServiceLoader.java
> ===================================================================
> --- spi/trunk/src/main/java/org/jboss/wsf/spi/util/ServiceLoader.java 2009-09-17 15:30:33 UTC (rev 10711)
> +++ spi/trunk/src/main/java/org/jboss/wsf/spi/util/ServiceLoader.java 2009-09-18 08:56:42 UTC (rev 10712)
> @@ -75,18 +75,26 @@
> {
> Object factory = null;
> String factoryName = null;
> - ClassLoader loader = SecurityActions.getContextClassLoader();
> + ClassLoader cl = SecurityActions.getContextClassLoader();
> + ResourceCachingClassLoader loader;
> +
> + if (cl instanceof ResourceCachingClassLoader)
> + {
> + loader = (ResourceCachingClassLoader)cl;
> + }
> + else
> + {
> + loader = new ResourceCachingClassLoader(cl);
> + SecurityActions.setContextClassLoader(loader);
> + }
>
> // Use the Services API (as detailed in the JAR specification), if available, to determine the classname.
> String filename = "META-INF/services/" + propertyName;
> - InputStream inStream = SecurityActions.getResourceAsStream(loader, filename);
> - if (inStream != null)
> + if (loader.hasResourceNameFromServices(filename))
> {
> + factoryName = loader.getResourceNameFromServices(filename);
> try
> {
> - BufferedReader br = new BufferedReader(new InputStreamReader(inStream, "UTF-8"));
> - factoryName = br.readLine();
> - br.close();
> if (factoryName != null)
> {
> Class factoryClass = SecurityActions.loadClass(loader, factoryName);
> @@ -98,6 +106,29 @@
> throw new IllegalStateException("Failed to load " + propertyName + ": " + factoryName, t);
> }
> }
> + else
> + {
> + InputStream inStream = SecurityActions.getResourceAsStream(loader, filename);
> + if (inStream != null)
> + {
> + try
> + {
> + BufferedReader br = new BufferedReader(new InputStreamReader(inStream, "UTF-8"));
> + factoryName = br.readLine();
> + br.close();
> + loader.setResourceNameFromServices(filename, factoryName);
> + if (factoryName != null)
> + {
> + Class factoryClass = SecurityActions.loadClass(loader, factoryName);
> + factory = factoryClass.newInstance();
> + }
> + }
> + catch (Throwable t)
> + {
> + throw new IllegalStateException("Failed to load " + propertyName + ": " + factoryName, t);
> + }
> + }
> + }
>
> // Use the default factory implementation class.
> if (factory == null&& defaultFactory != null)
>
> _______________________________________________
> jbossws-commits mailing list
> jbossws-commits at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jbossws-commits
--
Richard Opalka
JBoss Software Engineer
Mail: ropalka at redhat.com
Mobile: +420731186942
More information about the jboss-dev-forums
mailing list