1
0

[LOGGING-119] Prevent potential deadlock in WeakHashtable.

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/logging/trunk@1435077 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Thomas Neidhart
2013-01-18 10:51:35 +00:00
parent 5c81b031c3
commit ae4698230a
3 changed files with 19 additions and 5 deletions

View File

@@ -44,6 +44,9 @@ The <action> type attribute can be add,update,fix,remove.
</properties> </properties>
<body> <body>
<release version="1.1.2" date="In SVN" description="Bug fixes."> <release version="1.1.2" date="In SVN" description="Bug fixes.">
<action type="fix" issue="LOGGING-119">
Prevent potential deadlock scenario in WeakHashtable.
</action>
<action type="fix" issue="LOGGING-130"> <action type="fix" issue="LOGGING-130">
Potential missing privileged block for class loader. Potential missing privileged block for class loader.
</action> </action>

View File

@@ -19,11 +19,13 @@ package org.apache.commons.logging.impl;
import java.lang.ref.ReferenceQueue; import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference; import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Enumeration; import java.util.Enumeration;
import java.util.HashSet; import java.util.HashSet;
import java.util.Hashtable; import java.util.Hashtable;
import java.util.Iterator; import java.util.Iterator;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
@@ -217,7 +219,7 @@ public final class WeakHashtable extends Hashtable {
/** /**
*@see 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 // check for nulls, ensuring semantics match superclass
if (key == null) { if (key == null) {
throw new NullPointerException("Null keys are not allowed"); throw new NullPointerException("Null keys are not allowed");
@@ -265,7 +267,7 @@ public final class WeakHashtable extends Hashtable {
/** /**
*@see Hashtable *@see Hashtable
*/ */
public Object remove(Object key) { public synchronized Object remove(Object key) {
// for performance reasons, only purge every // for performance reasons, only purge every
// MAX_CHANGES_BEFORE_PURGE times // MAX_CHANGES_BEFORE_PURGE times
if (changeCount++ > MAX_CHANGES_BEFORE_PURGE) { if (changeCount++ > MAX_CHANGES_BEFORE_PURGE) {
@@ -317,12 +319,21 @@ public final class WeakHashtable extends Hashtable {
* have been garbage collected. * have been garbage collected.
*/ */
private void purge() { private void purge() {
final List toRemove = new ArrayList();
synchronized (queue) { synchronized (queue) {
WeakKey key; WeakKey key;
while ((key = (WeakKey) queue.poll()) != null) { 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));
}
} }
/** /**

View File

@@ -30,7 +30,7 @@ import java.util.Set;
import junit.framework.TestCase; 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 WAIT_FOR_THREAD_COMPLETION = 5000; // 5 seconds
private static final int RUN_LOOPS = 3000; private static final int RUN_LOOPS = 3000;
@@ -50,7 +50,7 @@ public class WeakHashtableTest extends TestCase {
private Long valueTwo; private Long valueTwo;
private Long valueThree; private Long valueThree;
public WeakHashtableTest(String testName) { public WeakHashtableTestCase(String testName) {
super(testName); super(testName);
} }