11:09 a.m.
Author: julien_viet
Date: 2011-10-03 12:09:34 -0400 (Mon, 03 Oct 2011)
New Revision: 7614
Modified:
portal/trunk/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java
portal/trunk/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java
Log:
GTNPORTAL-2148 : Detect reentrancy in future cache to prevent self deadlock
Modified:
portal/trunk/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java
===================================================================
---
portal/trunk/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java 2011-10-03
16:02:25 UTC (rev 7613)
+++
portal/trunk/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java 2011-10-03
16:09:34 UTC (rev 7614)
@@ -49,10 +49,10 @@
{
/** . */
- private final Loader<K, V, C> loader;
+ final Loader<K, V, C> loader;
/** . */
- private final ConcurrentMap<K, FutureTask<V>> futureEntries;
+ private final ConcurrentMap<K, Retrieval<K, V, C>> futureEntries;
/** . */
private final Logger log = LoggerFactory.getLogger(FutureCache.class);
@@ -60,7 +60,7 @@
public FutureCache(Loader<K, V, C> loader)
{
this.loader = loader;
- this.futureEntries = new ConcurrentHashMap<K, FutureTask<V>>();
+ this.futureEntries = new ConcurrentHashMap<K, Retrieval<K, V, C>>();
}
/**
@@ -99,65 +99,67 @@
if (value == null)
{
// Create our future
- FutureTask<V> future = new FutureTask<V>(new Callable<V>()
- {
- public V call() throws Exception
- {
- // Retrieve the value from the loader
- V value = loader.retrieve(context, key);
+ Retrieval<K, V, C> retrieval = new Retrieval<K, V, C>(context, key,
this);
- //
- if (value != null)
- {
- // Cache it, it is made available to other threads (unless someone
removes it)
- put(key, value);
-
- // Return value
- return value;
- }
- else
- {
- return null;
- }
- }
- });
-
// This boolean means we inserted in the local
boolean inserted = true;
//
try
{
- FutureTask<V> phantom = futureEntries.putIfAbsent(key, future);
+ Retrieval<K, V, C> phantom = futureEntries.putIfAbsent(key,
retrieval);
// Use the value that could have been inserted by another thread
if (phantom != null)
{
- future = phantom;
+ retrieval = phantom;
inserted = false;
}
else
{
- future.run();
+ try
+ {
+ retrieval.current = Thread.currentThread();
+ retrieval.future.run();
+ }
+ catch (Exception e)
+ {
+ log.error("Retrieval of resource " + key + " threw an
exception", e);
+ }
+ finally
+ {
+ retrieval.current = null;
+ }
}
// Returns the value
- value = future.get();
+ if (retrieval.current == Thread.currentThread())
+ {
+ throw new IllegalStateException("Reentrancy detected when obtaining
key " + key + " with context " + context + " detected");
+ }
+ else
+ {
+ try
+ {
+ value = retrieval.future.get();
+ }
+ catch (ExecutionException e)
+ {
+ log.error("Computing of resource " + key + " threw an
exception", e.getCause());
+ }
+ catch (InterruptedException e)
+ {
+ // We should handle interruped exception in some manner
+ log.error("Retrieval of resource " + key + " threw an
exception", e);
+ }
+ }
}
- catch (ExecutionException e)
- {
- log.error("Computing of resource " + key + " threw an
exception", e.getCause());
- }
- catch (Exception e)
- {
- log.error("Retrieval of resource " + key + " threw an
exception", e);
- }
finally
{
// Clean up the per key map but only if our insertion succeeded and with our
future
if (inserted)
{
- futureEntries.remove(key, future);
+ futureEntries.remove(key, retrieval);
}
}
}
Modified:
portal/trunk/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java
===================================================================
---
portal/trunk/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java 2011-10-03
16:02:25 UTC (rev 7613)
+++
portal/trunk/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java 2011-10-03
16:09:34 UTC (rev 7614)
@@ -23,6 +23,7 @@
import junit.framework.TestCase;
import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
/**
* @author <a href="mailto:julien.viet@exoplatform.com">Julien
Viet</a>
@@ -68,4 +69,32 @@
}, "foo"));
Assert.assertFalse(futureCache.data.containsKey("foo"));
}
+
+ public void testReentrancy()
+ {
+ final FutureMap<Callable<String>> futureCache = new
FutureMap<Callable<String>>(new StringLoader());
+ String res = futureCache.get(new Callable<String>()
+ {
+ public String call() throws Exception
+ {
+ try
+ {
+ futureCache.get(new Callable<String>()
+ {
+ public String call() throws Exception
+ {
+ // Should not go there
+ throw new AssertionError();
+ }
+ }, "foo");
+ return "fail";
+ }
+ catch (IllegalStateException expected)
+ {
+ return "pass";
+ }
+ }
+ }, "foo");
+ assertEquals("pass", res);
+ }
}