Author: bmaxwell Date: 2011-07-05 13:12:56 -0400 (Tue, 05 Jul 2011) New Revision: 1770 Modified: branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/javax/el/BeanELResolver.java branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/catalina/connector/Request.java branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/lang/ExpressionBuilder.java branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/util/ConcurrentCache.java Log: [JBPAPP-6778] JBWEB148: Possible NPE on set/removeAttribute, JBWEB185: Fix thread safety in two EL caches Modified: branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/javax/el/BeanELResolver.java =================================================================== --- branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/javax/el/BeanELResolver.java 2011-07-05 17:08:41 UTC (rev 1769) +++ branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/javax/el/BeanELResolver.java 2011-07-05 17:12:56 UTC (rev 1770) @@ -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/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/catalina/connector/Request.java =================================================================== --- branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/catalina/connector/Request.java 2011-07-05 17:08:41 UTC (rev 1769) +++ branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/catalina/connector/Request.java 2011-07-05 17:12:56 UTC (rev 1770) @@ -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/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/lang/ExpressionBuilder.java =================================================================== --- branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/lang/ExpressionBuilder.java 2011-07-05 17:08:41 UTC (rev 1769) +++ branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/lang/ExpressionBuilder.java 2011-07-05 17:12:56 UTC (rev 1770) @@ -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/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/util/ConcurrentCache.java =================================================================== --- branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/util/ConcurrentCache.java 2011-07-05 17:08:41 UTC (rev 1769) +++ branches/JBOSSWEB_2_1_10_GA_JBPAPP-6778/java/org/apache/el/util/ConcurrentCache.java 2011-07-05 17:12:56 UTC (rev 1770) @@ -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); + } }