Author: remy.maucherat(a)jboss.com Date: 2011-01-17 06:21:35 -0500 (Mon, 17 Jan 2011) New Revision: 1639 Modified: branches/2.1.x/java/javax/el/BeanELResolver.java branches/2.1.x/java/org/apache/catalina/connector/Request.java branches/2.1.x/java/org/apache/el/lang/ExpressionBuilder.java branches/2.1.x/java/org/apache/el/util/ConcurrentCache.java branches/2.1.x/webapps/docs/changelog.xml Log: - JBWEB-148: Possible NPE on set/removeAttribute. - JBWEB-185: Fix thread safety in two EL caches. Modified: branches/2.1.x/java/javax/el/BeanELResolver.java =================================================================== --- branches/2.1.x/java/javax/el/BeanELResolver.java 2011-01-07 14:17:42 UTC (rev 1638) +++ branches/2.1.x/java/javax/el/BeanELResolver.java 2011-01-17 11:21:35 UTC (rev 1639) @@ -25,6 +25,8 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.HashMap; import java.util.Iterator; @@ -34,10 +36,29 @@ public class BeanELResolver extends ELResolver { + private static final int CACHE_SIZE; + private static final String CACHE_SIZE_PROP = + "org.apache.el.BeanELResolver.CACHE_SIZE"; + + static { + if (System.getSecurityManager() == null) { + CACHE_SIZE = Integer.parseInt( + System.getProperty(CACHE_SIZE_PROP, "1000")); + } else { + CACHE_SIZE = AccessController.doPrivileged( + new PrivilegedAction<Integer>() { + public Integer run() { + return Integer.valueOf( + System.getProperty(CACHE_SIZE_PROP, "1000")); + } + }).intValue(); + } + } + private final boolean readOnly; - private final ConcurrentCache<String, BeanProperties> cache = new ConcurrentCache<String, BeanProperties>( - 1000); + private final ConcurrentCache<String, BeanProperties> cache = + new ConcurrentCache<String, BeanProperties>(CACHE_SIZE); public BeanELResolver() { this.readOnly = false; @@ -324,7 +345,9 @@ public V get(K key) { V value = this.eden.get(key); if (value == null) { - value = this.longterm.get(key); + synchronized (longterm) { + value = this.longterm.get(key); + } if (value != null) { this.eden.put(key, value); } @@ -334,11 +357,12 @@ public void put(K key, V value) { if (this.eden.size() >= this.size) { - this.longterm.putAll(this.eden); + synchronized (longterm) { + this.longterm.putAll(this.eden); + } this.eden.clear(); } this.eden.put(key, value); } - } } Modified: branches/2.1.x/java/org/apache/catalina/connector/Request.java =================================================================== --- branches/2.1.x/java/org/apache/catalina/connector/Request.java 2011-01-07 14:17:42 UTC (rev 1638) +++ branches/2.1.x/java/org/apache/catalina/connector/Request.java 2011-01-17 11:21:35 UTC (rev 1639) @@ -1369,6 +1369,9 @@ return; } + if (context == null) + return; + // Notify interested application event listeners Object listeners[] = context.getApplicationEventListeners(); if ((listeners == null) || (listeners.length == 0)) @@ -1439,6 +1442,9 @@ coyoteRequest.setAttribute(name, value); } + if (context == null) + return; + // Notify interested application event listeners Object listeners[] = context.getApplicationEventListeners(); if ((listeners == null) || (listeners.length == 0)) Modified: branches/2.1.x/java/org/apache/el/lang/ExpressionBuilder.java =================================================================== --- branches/2.1.x/java/org/apache/el/lang/ExpressionBuilder.java 2011-01-07 14:17:42 UTC (rev 1638) +++ branches/2.1.x/java/org/apache/el/lang/ExpressionBuilder.java 2011-01-17 11:21:35 UTC (rev 1639) @@ -50,7 +50,7 @@ */ public final class ExpressionBuilder implements NodeVisitor { - private static final ConcurrentCache cache = new ConcurrentCache(5000); + private static final ConcurrentCache cache = new ConcurrentCache(10000); private FunctionMapper fnMapper; Modified: branches/2.1.x/java/org/apache/el/util/ConcurrentCache.java =================================================================== --- branches/2.1.x/java/org/apache/el/util/ConcurrentCache.java 2011-01-07 14:17:42 UTC (rev 1638) +++ branches/2.1.x/java/org/apache/el/util/ConcurrentCache.java 2011-01-17 11:21:35 UTC (rev 1639) @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.apache.el.util; import java.util.Map; @@ -6,34 +22,38 @@ public final class ConcurrentCache<K,V> { - private final int size; - - private final Map<K,V> eden; - - private final Map<K,V> longterm; - - public ConcurrentCache(int size) { - this.size = size; - this.eden = new ConcurrentHashMap<K,V>(size); - this.longterm = new WeakHashMap<K,V>(size); - } - - public V get(K k) { - V v = this.eden.get(k); - if (v == null) { - v = this.longterm.get(k); - if (v != null) { - this.eden.put(k, v); - } - } - return v; - } - - public void put(K k, V v) { - if (this.eden.size() >= size) { - this.longterm.putAll(this.eden); - this.eden.clear(); - } - this.eden.put(k, v); - } + private final int size; + + private final Map<K,V> eden; + + private final Map<K,V> longterm; + + public ConcurrentCache(int size) { + this.size = size; + this.eden = new ConcurrentHashMap<K,V>(size); + this.longterm = new WeakHashMap<K,V>(size); + } + + public V get(K k) { + V v = this.eden.get(k); + if (v == null) { + synchronized (longterm) { + v = this.longterm.get(k); + } + if (v != null) { + this.eden.put(k, v); + } + } + return v; + } + + public void put(K k, V v) { + if (this.eden.size() >= size) { + synchronized (longterm) { + this.longterm.putAll(this.eden); + } + this.eden.clear(); + } + this.eden.put(k, v); + } } Modified: branches/2.1.x/webapps/docs/changelog.xml =================================================================== --- branches/2.1.x/webapps/docs/changelog.xml 2011-01-07 14:17:42 UTC (rev 1638) +++ branches/2.1.x/webapps/docs/changelog.xml 2011-01-17 11:21:35 UTC (rev 1639) @@ -16,6 +16,13 @@ <body> <section name="JBoss Web 2.1.11.GA (remm)"> + <subsection name="Catalina"> + <changelog> + <fix> + <jira>148</jira>: Fix possible NPE on request.set/removeAttribute. (remm) + </fix> + </changelog> + </subsection> <subsection name="Coyote"> <changelog> <fix> @@ -23,6 +30,14 @@ </fix> </changelog> </subsection> + <subsection name="Jasper"> + <changelog> + <fix> + <jira>185</jira>: Fix cache thread safety issue for EL expression builder. + Patch submitted by Takayoshi Kimura. (remm) + </fix> + </changelog> + </subsection> </section> <section name="JBoss Web 2.1.10.GA (remm)">