You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2012/07/23 12:54:09 UTC
git commit: WICKET-4646 atomicity violation bugs of using concurrent
collections
Updated Branches:
refs/heads/wicket-1.5.x 29844aa3f -> 0fdae2e8e
WICKET-4646 atomicity violation bugs of using concurrent collections
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/0fdae2e8
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/0fdae2e8
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/0fdae2e8
Branch: refs/heads/wicket-1.5.x
Commit: 0fdae2e8eb3c8716409e4c69e22deea902c3f6fd
Parents: 29844aa
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Mon Jul 23 13:53:48 2012 +0300
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Mon Jul 23 13:53:48 2012 +0300
----------------------------------------------------------------------
.../apache/wicket/feedback/FeedbackMessages.java | 17 +++++---
.../request/resource/PackageResourceReference.java | 6 ++-
.../apache/wicket/session/DefaultPageFactory.java | 33 ++++++++-------
.../apache/wicket/util/lang/PropertyResolver.java | 11 +++--
.../wicket/versioning/InMemoryPageStore.java | 9 +++-
.../annot/AnnotProxyFieldValueFactory.java | 14 +++++-
.../converter/AbstractIntegerConverter.java | 9 +++-
.../wicket/util/watch/ModificationWatcher.java | 7 +--
8 files changed, 66 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessages.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessages.java b/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessages.java
index 5385043..b33c967 100644
--- a/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessages.java
+++ b/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessages.java
@@ -68,11 +68,12 @@ public final class FeedbackMessages implements IClusterable, Iterable<FeedbackMe
*/
public final void add(FeedbackMessage message)
{
- if (log.isDebugEnabled())
+ log.debug("Adding feedback message '{}'", message);
+
+ synchronized (messages)
{
- log.debug("Adding feedback message " + message);
+ messages.add(message);
}
- messages.add(message);
}
/**
@@ -197,9 +198,13 @@ public final class FeedbackMessages implements IClusterable, Iterable<FeedbackMe
message.detach();
}
- messages.removeAll(toDelete);
-
- return toDelete.size();
+ synchronized(messages)
+ {
+ int sizeBefore = messages.size();
+ messages.removeAll(toDelete);
+ int sizeAfter = messages.size();
+ return sizeAfter - sizeBefore;
+ }
}
/**
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
index 4d21c94..d422082 100644
--- a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
+++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
@@ -153,7 +153,11 @@ public class PackageResourceReference extends ResourceReference
if (value == null)
{
value = getUrlAttributes(locale, style, variation);
- urlAttributesCacheMap.put(key, value);
+ UrlAttributes tmpValue = urlAttributesCacheMap.putIfAbsent(key, value);
+ if (tmpValue != null)
+ {
+ value = tmpValue;
+ }
}
return value;
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-core/src/main/java/org/apache/wicket/session/DefaultPageFactory.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/session/DefaultPageFactory.java b/wicket-core/src/main/java/org/apache/wicket/session/DefaultPageFactory.java
index 572fc4c..224ea86 100644
--- a/wicket-core/src/main/java/org/apache/wicket/session/DefaultPageFactory.java
+++ b/wicket-core/src/main/java/org/apache/wicket/session/DefaultPageFactory.java
@@ -18,7 +18,6 @@ package org.apache.wicket.session;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
-import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@@ -50,7 +49,7 @@ public final class DefaultPageFactory implements IPageFactory
private static final Logger log = LoggerFactory.getLogger(DefaultPageFactory.class);
/** Map of Constructors for Page subclasses */
- private final Map<Class<?>, Constructor<?>> constructorForClass = Generics.newConcurrentHashMap();
+ private final ConcurrentMap<Class<?>, Constructor<?>> constructorForClass = Generics.newConcurrentHashMap();
/**
* {@link #isBookmarkable(Class)} is expensive, we cache the result here
@@ -113,7 +112,7 @@ public final class DefaultPageFactory implements IPageFactory
* @return The page constructor, or null if no one-arg constructor can be found taking the given
* argument type.
*/
- private final <C extends IRequestablePage> Constructor<?> constructor(final Class<C> pageClass,
+ private <C extends IRequestablePage> Constructor<?> constructor(final Class<C> pageClass,
final Class<PageParameters> argumentType)
{
// Get constructor for page class from cache
@@ -128,22 +127,20 @@ public final class DefaultPageFactory implements IPageFactory
constructor = pageClass.getConstructor(new Class[] { argumentType });
// Store it in the cache
- constructorForClass.put(pageClass, constructor);
-
- if (log.isDebugEnabled())
+ Constructor<?> tmpConstructor = constructorForClass.putIfAbsent(pageClass, constructor);
+ if (tmpConstructor != null)
{
- log.debug("Found constructor for Page of type '{}' and argument of type '{}'.",
- pageClass, argumentType);
+ constructor = tmpConstructor;
}
+
+ log.debug("Found constructor for Page of type '{}' and argument of type '{}'.",
+ pageClass, argumentType);
}
catch (NoSuchMethodException e)
{
- if (log.isDebugEnabled())
- {
- log.debug(
- "Page of type '{}' has not visible constructor with an argument of type '{}'.",
- pageClass, argumentType);
- }
+ log.debug(
+ "Page of type '{}' has not visible constructor with an argument of type '{}'.",
+ pageClass, argumentType);
return null;
}
@@ -164,7 +161,7 @@ public final class DefaultPageFactory implements IPageFactory
* Thrown if the Page cannot be instantiated using the given constructor and
* argument.
*/
- private final Page newPage(final Constructor<?> constructor, final Object argument)
+ private Page newPage(final Constructor<?> constructor, final Object argument)
{
try
{
@@ -257,7 +254,11 @@ public final class DefaultPageFactory implements IPageFactory
{
bookmarkable = Boolean.FALSE;
}
- pageToBookmarkableCache.put(pageClass.getName(), bookmarkable);
+ Boolean tmpBookmarkable = pageToBookmarkableCache.putIfAbsent(pageClass.getName(), bookmarkable);
+ if (tmpBookmarkable != null)
+ {
+ bookmarkable = tmpBookmarkable;
+ }
}
return bookmarkable;
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-core/src/main/java/org/apache/wicket/util/lang/PropertyResolver.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/util/lang/PropertyResolver.java b/wicket-core/src/main/java/org/apache/wicket/util/lang/PropertyResolver.java
index a6471f4..893e1d4c 100644
--- a/wicket-core/src/main/java/org/apache/wicket/util/lang/PropertyResolver.java
+++ b/wicket-core/src/main/java/org/apache/wicket/util/lang/PropertyResolver.java
@@ -28,7 +28,6 @@ import org.apache.wicket.Application;
import org.apache.wicket.Session;
import org.apache.wicket.WicketRuntimeException;
import org.apache.wicket.util.convert.ConversionException;
-import org.apache.wicket.util.lang.PropertyResolver.IClassCache;
import org.apache.wicket.util.string.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -71,7 +70,7 @@ public final class PropertyResolver
private final static int CREATE_NEW_VALUE = 1;
private final static int RESOLVE_CLASS = 2;
- private final static Map<Object, IClassCache> applicationToClassesToGetAndSetters = Generics.newConcurrentHashMap(2);
+ private final static ConcurrentHashMap<Object, IClassCache> applicationToClassesToGetAndSetters = Generics.newConcurrentHashMap(2);
private static final String GET = "get";
private static final String IS = "is";
@@ -1416,7 +1415,7 @@ public final class PropertyResolver
private static IClassCache getClassesToGetAndSetters()
{
- Object key = null;
+ Object key;
if (Application.exists())
{
key = Application.get();
@@ -1428,7 +1427,11 @@ public final class PropertyResolver
IClassCache result = applicationToClassesToGetAndSetters.get(key);
if (result == null)
{
- applicationToClassesToGetAndSetters.put(key, result = new DefaultClassCache());
+ IClassCache tmpResult = applicationToClassesToGetAndSetters.putIfAbsent(key, result = new DefaultClassCache());
+ if (tmpResult != null)
+ {
+ result = tmpResult;
+ }
}
return result;
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java b/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java
index 0087666..c6a5642 100644
--- a/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java
+++ b/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java
@@ -16,7 +16,6 @@
*/
package org.apache.wicket.versioning;
-import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -34,7 +33,7 @@ public class InMemoryPageStore implements IDataStore
/**
* A map of : sessionId => pageId => pageAsBytes
*/
- private final Map<String, Map<Integer, byte[]>> store;
+ private final ConcurrentHashMap<String, Map<Integer, byte[]>> store;
/**
* Construct.
@@ -94,7 +93,11 @@ public class InMemoryPageStore implements IDataStore
if (sessionPages == null)
{
sessionPages = new ConcurrentHashMap<Integer, byte[]>();
- store.put(sessionId, sessionPages);
+ Map<Integer, byte[]> tmpSessionPages = store.putIfAbsent(sessionId, sessionPages);
+ if (tmpSessionPages != null)
+ {
+ sessionPages = tmpSessionPages;
+ }
}
sessionPages.put(pageId, pageAsBytes);
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java
----------------------------------------------------------------------
diff --git a/wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java b/wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java
index 9d37847..8abbfd6 100644
--- a/wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java
+++ b/wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java
@@ -126,7 +126,7 @@ public class AnnotProxyFieldValueFactory implements IFieldValueFactory
return cachedValue;
}
- final Object target;
+ Object target;
if (wrapInProxies)
{
target = LazyInitProxyFactory.createProxy(field.getType(), locator);
@@ -139,7 +139,11 @@ public class AnnotProxyFieldValueFactory implements IFieldValueFactory
// only put the proxy into the cache if the bean is a singleton
if (locator.isSingletonBean())
{
- cache.put(locator, target);
+ Object tmpTarget = cache.putIfAbsent(locator, target);
+ if (tmpTarget != null)
+ {
+ target = tmpTarget;
+ }
}
return target;
}
@@ -165,7 +169,11 @@ public class AnnotProxyFieldValueFactory implements IFieldValueFactory
if (name != null)
{
- beanNameCache.put(field.getType(), name);
+ String tmpName = beanNameCache.putIfAbsent(field.getType(), name);
+ if (tmpName != null)
+ {
+ name = tmpName;
+ }
}
}
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-util/src/main/java/org/apache/wicket/util/convert/converter/AbstractIntegerConverter.java
----------------------------------------------------------------------
diff --git a/wicket-util/src/main/java/org/apache/wicket/util/convert/converter/AbstractIntegerConverter.java b/wicket-util/src/main/java/org/apache/wicket/util/convert/converter/AbstractIntegerConverter.java
index 6998fde..5b3d864 100644
--- a/wicket-util/src/main/java/org/apache/wicket/util/convert/converter/AbstractIntegerConverter.java
+++ b/wicket-util/src/main/java/org/apache/wicket/util/convert/converter/AbstractIntegerConverter.java
@@ -18,7 +18,6 @@ package org.apache.wicket.util.convert.converter;
import java.text.NumberFormat;
import java.util.Locale;
-import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
/**
@@ -32,7 +31,7 @@ public abstract class AbstractIntegerConverter<I extends Number> extends Abstrac
private static final long serialVersionUID = 1L;
/** The date format to use */
- private final Map<Locale, NumberFormat> numberFormats = new ConcurrentHashMap<Locale, NumberFormat>();
+ private final ConcurrentHashMap<Locale, NumberFormat> numberFormats = new ConcurrentHashMap<Locale, NumberFormat>();
/**
* @param locale
@@ -48,7 +47,11 @@ public abstract class AbstractIntegerConverter<I extends Number> extends Abstrac
numberFormat = NumberFormat.getIntegerInstance(locale);
numberFormat.setParseIntegerOnly(true);
numberFormat.setGroupingUsed(false);
- numberFormats.put(locale, numberFormat);
+ NumberFormat tmpNumberFormat = numberFormats.putIfAbsent(locale, numberFormat);
+ if (tmpNumberFormat != null)
+ {
+ numberFormat = tmpNumberFormat;
+ }
}
return (NumberFormat)numberFormat.clone();
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/0fdae2e8/wicket-util/src/main/java/org/apache/wicket/util/watch/ModificationWatcher.java
----------------------------------------------------------------------
diff --git a/wicket-util/src/main/java/org/apache/wicket/util/watch/ModificationWatcher.java b/wicket-util/src/main/java/org/apache/wicket/util/watch/ModificationWatcher.java
index 8abe543..8662eaa 100644
--- a/wicket-util/src/main/java/org/apache/wicket/util/watch/ModificationWatcher.java
+++ b/wicket-util/src/main/java/org/apache/wicket/util/watch/ModificationWatcher.java
@@ -16,7 +16,6 @@
*/
package org.apache.wicket.util.watch;
-import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -43,7 +42,7 @@ public class ModificationWatcher implements IModificationWatcher
private static final Logger log = LoggerFactory.getLogger(ModificationWatcher.class);
/** maps <code>IModifiable</code> objects to <code>Entry</code> objects */
- private final Map<IModifiable, Entry> modifiableToEntry = new ConcurrentHashMap<IModifiable, Entry>();
+ private final ConcurrentHashMap<IModifiable, Entry> modifiableToEntry = new ConcurrentHashMap<IModifiable, Entry>();
/** the <code>Task</code> to run */
private Task task;
@@ -104,12 +103,12 @@ public class ModificationWatcher implements IModificationWatcher
newEntry.listeners.add(listener);
// Put in map
- modifiableToEntry.put(modifiable, newEntry);
+ modifiableToEntry.putIfAbsent(modifiable, newEntry);
}
else
{
// The IModifiable is not returning a valid lastModifiedTime
- log.info("Cannot track modifications to resource " + modifiable);
+ log.info("Cannot track modifications to resource '{}'", modifiable);
}
return true;