diff --git a/src/changes/changes.xml b/src/changes/changes.xml index b93038e..a72f700 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -44,6 +44,9 @@ The type attribute can be add,update,fix,remove. + + Prevent potential deadlock scenario in WeakHashtable. + Potential missing privileged block for class loader. diff --git a/src/main/java/org/apache/commons/logging/impl/WeakHashtable.java b/src/main/java/org/apache/commons/logging/impl/WeakHashtable.java index 3c5e812..0c65975 100644 --- a/src/main/java/org/apache/commons/logging/impl/WeakHashtable.java +++ b/src/main/java/org/apache/commons/logging/impl/WeakHashtable.java @@ -19,11 +19,13 @@ package org.apache.commons.logging.impl; import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; +import java.util.ArrayList; import java.util.Collection; import java.util.Enumeration; import java.util.HashSet; import java.util.Hashtable; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; @@ -217,7 +219,7 @@ public final class WeakHashtable extends Hashtable { /** *@see Hashtable */ - public Object put(Object key, Object value) { + public synchronized Object put(Object key, Object value) { // check for nulls, ensuring semantics match superclass if (key == null) { throw new NullPointerException("Null keys are not allowed"); @@ -265,7 +267,7 @@ public final class WeakHashtable extends Hashtable { /** *@see Hashtable */ - public Object remove(Object key) { + public synchronized Object remove(Object key) { // for performance reasons, only purge every // MAX_CHANGES_BEFORE_PURGE times if (changeCount++ > MAX_CHANGES_BEFORE_PURGE) { @@ -317,12 +319,21 @@ public final class WeakHashtable extends Hashtable { * have been garbage collected. */ private void purge() { + final List toRemove = new ArrayList(); synchronized (queue) { WeakKey key; while ((key = (WeakKey) queue.poll()) != null) { - super.remove(key.getReferenced()); + toRemove.add(key.getReferenced()); } } + + // LOGGING-119: do the actual removal of the keys outside the sync block + // to prevent deadlock scenarios as purge() may be called from + // non-synchronized methods too + final int size = toRemove.size(); + for (int i = 0; i < size; i++) { + super.remove(toRemove.get(i)); + } } /** diff --git a/src/test/java/org/apache/commons/logging/impl/WeakHashtableTest.java b/src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java similarity index 99% rename from src/test/java/org/apache/commons/logging/impl/WeakHashtableTest.java rename to src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java index b77ecc9..64a28f5 100644 --- a/src/test/java/org/apache/commons/logging/impl/WeakHashtableTest.java +++ b/src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java @@ -30,7 +30,7 @@ import java.util.Set; import junit.framework.TestCase; -public class WeakHashtableTest extends TestCase { +public class WeakHashtableTestCase extends TestCase { private static final int WAIT_FOR_THREAD_COMPLETION = 5000; // 5 seconds private static final int RUN_LOOPS = 3000; @@ -50,7 +50,7 @@ public class WeakHashtableTest extends TestCase { private Long valueTwo; private Long valueThree; - public WeakHashtableTest(String testName) { + public WeakHashtableTestCase(String testName) { super(testName); }