Bug 448472 - Cycle while computing value LegacyHandlerService
ValueComputation.get() can be accessed from multiple threads, but has no
protection against multi-threaded access. One example of this is looking
up the handler of a command via HandlerServiceImpl.lookUpHandler(), from
a job. E.g. when closing a project, the CloseUnrelatedProjectsAction
will react to a resource change event, potentially looking up a handler
simultaneously to selection change handling for other Package Explorer
actions. One of the possible results is that an exception is thrown,
that complains about cycles.
This change retains the delegated computation without synchronization,
but storing the cached value is done in a synchronized block.
Furthermore the cycle detection is improved, so that concurrent access
is not reported as a cycle. This is done by walking the (thread local)
computation stack of the current context and looking for the current
computation, but only if the current computation is already ongoing upon
method entry; i.e. only in cases of concurrent access or actual cycles.
Change-Id: I0d211c22df0c5650922e09fb93e6aa6b9a8aff62
Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com>
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
index 43b46d6..5a0f644 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
@@ -767,6 +767,12 @@
current.push(comp);
}
+ public boolean hasComputation(Computation comp) {
+ Stack<Computation> current = getCalculatedComputations();
+ boolean hasComputation = current.contains(comp);
+ return hasComputation;
+ }
+
public void popComputation(Computation comp) {
Stack<Computation> current = getCalculatedComputations();
Computation ended = current.pop();
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java
index 42ceadf..9e51e96 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ValueComputation.java
@@ -29,7 +29,7 @@
final private String name;
private Object cachedValue = NotAValue;
- private boolean computing; // cycle detection
+ private volatile boolean computing; // cycle detection
private boolean valid = true;
public ValueComputation(String name, IEclipseContext originatingContext, IContextFunction computedValue) {
@@ -57,13 +57,18 @@
public Object get() {
if (cachedValue != NotAValue)
return cachedValue;
- if (this.computing)
- throw new RuntimeException("Cycle while computing value " + this); //$NON-NLS-1$
+ if (computing) {
+ boolean hasCycle = originatingContext.hasComputation(this);
+ if (hasCycle) {
+ throw new RuntimeException("Cycle while computing value " + this); //$NON-NLS-1$
+ }
+ }
originatingContext.pushComputation(this);
computing = true;
try {
- cachedValue = function.compute(originatingContext, name);
+ Object computed = function.compute(originatingContext, name);
+ cacheComputedValue(computed);
} finally {
computing = false;
originatingContext.popComputation(this);
@@ -71,6 +76,12 @@
return cachedValue;
}
+ private synchronized void cacheComputedValue(Object computed) {
+ if (cachedValue == NotAValue) {
+ cachedValue = computed;
+ }
+ }
+
@Override
public String toString() {
if (function == null)
