This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "Logback: the generic, reliable, fast and flexible logging framework.". The branch, master has been updated via ebb58bb6ea3816a4e4ffd805b722c8121bba61a6 (commit) from 5be4443a22e7f8b886b867a29c141432179c1d53 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- http://git.qos.ch/gitweb/?p=logback.git;a=commit;h=ebb58bb6ea3816a4e4ffd805b... http://github.com/ceki/logback/commit/ebb58bb6ea3816a4e4ffd805b722c8121bba61... commit ebb58bb6ea3816a4e4ffd805b722c8121bba61a6 Author: Ceki Gulcu <ceki@qos.ch> Date: Fri Mar 18 15:14:14 2011 +0100 fixing LBCLASSIC-255 diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java b/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java index cfa963f..7715f66 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/turbo/LRUMessageCache.java @@ -13,17 +13,19 @@ */ package ch.qos.logback.classic.turbo; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; +/** + * Clients of this class should only use the {@link #getMessageCountAndThenIncrement} method. Other methods inherited + * via LinkedHashMap are not thread safe. + */ class LRUMessageCache extends LinkedHashMap<String, Integer> { - // LinkedHashMap permits null elements to be inserted - private static final long serialVersionUID = 1L; - final int cacheSize; - + LRUMessageCache(int cacheSize) { super((int) (cacheSize * (4.0f / 3)), 0.75f, true); if (cacheSize < 1) { @@ -31,24 +33,35 @@ class LRUMessageCache extends LinkedHashMap<String, Integer> { } this.cacheSize = cacheSize; } - + int getMessageCountAndThenIncrement(String msg) { // don't insert null elements - if(msg == null) { + if (msg == null) { return 0; } - - Integer i = super.get(msg); - if(i == null) { - i = 0; - } else { - i = new Integer(i.intValue()+1); + + Integer i; + // LinkedHashMap is not LinkedHashMap. See also LBCLASSIC-255 + synchronized (this) { + i = super.get(msg); + if (i == null) { + i = 0; + } else { + i = new Integer(i.intValue() + 1); + } + super.put(msg, i); } - super.put(msg, i); return i; } - + + // called indirectly by get() or put() which are already supposed to be + // called from within a synchronized block protected boolean removeEldestEntry(Map.Entry eldest) { return (size() > cacheSize); } + + @Override + synchronized public void clear() { + super.clear(); + } } diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/turbo/LRUMessageCacheTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/LRUMessageCacheTest.java index 5e2fed3..453d0b8 100644 --- a/logback-classic/src/test/java/ch/qos/logback/classic/turbo/LRUMessageCacheTest.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/LRUMessageCacheTest.java @@ -13,8 +13,6 @@ import junit.framework.Assert; import org.junit.Test; public class LRUMessageCacheTest { - private static final int INVOCATIONS_PER_TASK = 500 * 1024; - private static final int THREADS_NUMBER = 16; @Test public void testEldestEntriesRemoval() { @@ -35,46 +33,4 @@ public class LRUMessageCacheTest { Assert.assertEquals(0, cache.getMessageCountAndThenIncrement("2")); } - @Test - public void multiThreadsTest() throws InterruptedException, ExecutionException { - final LRUMessageCache cache = new LRUMessageCache(THREADS_NUMBER); - - ArrayList<TestTask> tasks = new ArrayList<TestTask>(THREADS_NUMBER); - for (int i = 0; i < THREADS_NUMBER; i++) { - tasks.add(new TestTask(cache)); - } - - ExecutorService execSrv = Executors.newFixedThreadPool(THREADS_NUMBER); - - List<Future<Boolean>> futures = execSrv.invokeAll(tasks); - for (Future<Boolean> future : futures) { - // Validate that task has successfully finished. - future.get(); - } - } - - /** - * Each thread is using always the same "Message" key. - */ - private class TestTask implements Callable<Boolean> { - private int prevValue = -1; - private final LRUMessageCache cache; - - public TestTask(LRUMessageCache cache) { - this.cache = cache; - } - - public Boolean call() throws Exception { - String msg = Thread.currentThread().getName(); - - for (int i = 0; i < INVOCATIONS_PER_TASK; i++) { - int current = cache.getMessageCountAndThenIncrement(msg); - // Ensure that new count is greater than previous count. - Assert.assertEquals(prevValue + 1, current); - prevValue = current; - } - - return true; - } - } } diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_Entry.java b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_Entry.java index 980ab2e..45ee86a 100644 --- a/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_Entry.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_Entry.java @@ -17,7 +17,7 @@ public class T_Entry<K> implements Comparable { K k; long sequenceNumber; - + T_Entry(K k, long sn) { this.k = k; this.sequenceNumber = sn; diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_LRUCache.java b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_LRUCache.java index ebb7150..36d56b3 100644 --- a/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_LRUCache.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/T_LRUCache.java @@ -79,7 +79,8 @@ public class T_LRUCache<K> { } return null; } - + + public void dump() { System.out.print("T:"); for (T_Entry<K> te : cacheList) { diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/X_LRUCache.java b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/X_LRUCache.java new file mode 100644 index 0000000..1a51573 --- /dev/null +++ b/logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/X_LRUCache.java @@ -0,0 +1,51 @@ +/** + * Logback: the reliable, generic, fast and flexible logging framework. + * Copyright (C) 1999-2009, QOS.ch. All rights reserved. + * + * This program and the accompanying materials are dual-licensed under + * either the terms of the Eclipse Public License v1.0 as published by + * the Eclipse Foundation + * + * or (per the licensee's choosing) + * + * under the terms of the GNU Lesser General Public License version 2.1 + * as published by the Free Software Foundation. + */ +package ch.qos.logback.classic.turbo.lru; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * An lru cache based on Java's LinkedHashMap. + * + * @author Ceki Gulcu + * + * @param <K> + * @param <V> + */ +public class X_LRUCache<K, V> extends LinkedHashMap<K, V> { + private static final long serialVersionUID = -6592964689843698200L; + + final int cacheSize; + + public X_LRUCache(int cacheSize) { + super((int) (cacheSize*(4.0f/3)), 0.75f, true); + if(cacheSize < 1) { + throw new IllegalArgumentException("Cache size cannnot be smaller than 1"); + } + this.cacheSize = cacheSize; + } + + protected boolean removeEldestEntry(Map.Entry eldest) { + return (size() > cacheSize); + } + + List<K> keyList() { + ArrayList<K> al = new ArrayList<K>(); + al.addAll(keySet()); + return al; + } +} diff --git a/logback-site/src/site/pages/news.html b/logback-site/src/site/pages/news.html index 2cb6553..13d7670 100644 --- a/logback-site/src/site/pages/news.html +++ b/logback-site/src/site/pages/news.html @@ -55,6 +55,10 @@ href="http://jira.qos.ch/browse/LBCORE-193">LBCORE-193</a> reported by B. K. Oxley.</p> + <p>Fixed thread safety issue in LRUMessageCache reported in <a + href="http://jira.qos.ch/browse/LBCLASSIC-255">LBCLASSIC-255</a> + by César Álvarez Núñez. + </p> <hr width="80%" align="center" /> ----------------------------------------------------------------------- Summary of changes: .../qos/logback/classic/turbo/LRUMessageCache.java | 41 +++++--- .../logback/classic/turbo/LRUMessageCacheTest.java | 44 --------- .../ch/qos/logback/classic/turbo/lru/T_Entry.java | 2 +- .../qos/logback/classic/turbo/lru/T_LRUCache.java | 3 +- .../turbo/lru/{LRUCache.java => X_LRUCache.java} | 102 ++++++++++---------- logback-site/src/site/pages/news.html | 4 + 6 files changed, 85 insertions(+), 111 deletions(-) copy logback-classic/src/test/java/ch/qos/logback/classic/turbo/lru/{LRUCache.java => X_LRUCache.java} (89%) hooks/post-receive -- Logback: the generic, reliable, fast and flexible logging framework.