diff --git a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java b/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java index 25942e12b44121cf14f76f398ab976b37b50a02e..0fda16012fdc0afcfda73d3623dc56e4b9cf0f2f 100644 --- a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java +++ b/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java @@ -236,6 +236,11 @@ public final class Evaluator interpreter.set(name, value); } + public Object get(String name) + { + return interpreter.get(name); + } + /** * Deletes the variable <var>name</var> from the evaluator's name space. */ diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java index cb7a6d0009336f2730a13afd49ab1fedf9e1663d..b72541f75a3b28016d33d3411a858fc819ee1ac4 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java @@ -16,9 +16,10 @@ package ch.systemsx.cisd.openbis.generic.server; -import java.util.Collection; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Stack; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -36,9 +37,8 @@ import ch.systemsx.cisd.openbis.generic.shared.managed_property.IEvaluationRunne import ch.systemsx.cisd.openbis.generic.shared.managed_property.ManagedPropertyFunctions; /** - * Pool of Jython Evaluators for managed properties. When the pool is created, it is filled with new - * Evaluator instances for every managed property script. Enables thread-safe execution of calls to - * python functions defined in these scripts. + * Pool of Jython Evaluators for managed properties. When the pool is created, it is filled with new Evaluator instances for every managed property + * script. Enables thread-safe execution of calls to python functions defined in these scripts. * * @author anttil */ @@ -55,6 +55,8 @@ public class JythonEvaluatorPool private Lock cacheLock; + private static ThreadLocal<Stack<Map<String, Object>>> stack = new ThreadLocal<Stack<Map<String, Object>>>(); + public JythonEvaluatorPool(IDAOFactory daoFactory, String poolSize) { this(daoFactory, createCache(poolSize)); @@ -107,9 +109,8 @@ public class JythonEvaluatorPool } /** - * Evaluate python functions from a script using an Evaluator instance in the pool. If the - * Evaluator instance does not exist, create it. Give access to the instance for only one thread - * at a time. + * Evaluate python functions from a script using an Evaluator instance in the pool. If the Evaluator instance does not exist, create it. Give + * access to the instance for only one thread at a time. */ private <T> T evaluate(String expression, Class<?> clazz, String script, IAtomicEvaluation<T> evaluation) @@ -134,14 +135,27 @@ public class JythonEvaluatorPool } } - Lock lock = state.getLock(); + ReentrantLock lock = (ReentrantLock) state.getLock(); + + if (false == lock.isHeldByCurrentThread()) + { + stack.set(new Stack<Map<String, Object>>()); + } + lock.lock(); + state.push(); + try { - return evaluation.evaluate(state.getCleanInstance()); + return evaluation.evaluate(state.getEvaluator()); } finally { + state.pop(); lock.unlock(); + if (false == lock.isHeldByCurrentThread()) + { + stack.set(null); + } } } @@ -168,37 +182,52 @@ public class JythonEvaluatorPool } /** - * The pooled object. Contains the Evaluator and its initial state to which it is always - * returned to when a new evaluation is started. + * The pooled object. Contains the Evaluator and its initial state to which it is always returned to when a new evaluation is started. */ public static class EvaluatorState { private final Evaluator evaluator; - private final Collection<String> globals; + private Stack<Map<String, Object>> globalsStack; private final Lock lock; public EvaluatorState(Evaluator evaluator) { this.evaluator = evaluator; - this.globals = evaluator.getGlobalVariables(); this.lock = new ReentrantLock(); + this.globalsStack = new Stack<Map<String, Object>>(); } - /** - * Returns an evaluator instance in its initial state. - */ - public synchronized Evaluator getCleanInstance() + public void push() { - for (String value : evaluator.getGlobalVariables()) + Map<String, Object> globalsValues = new HashMap<String, Object>(); + for (String globalName : evaluator.getGlobalVariables()) { - if (globals.contains(value) == false) - { - evaluator.delete(value); - } + Object globalValue = evaluator.get(globalName); + globalsValues.put(globalName, globalValue); + } + globalsStack.push(globalsValues); + } + + public void pop() + { + Map<String, Object> globalsValues = globalsStack.pop(); + + for (String globalName : evaluator.getGlobalVariables()) + { + evaluator.delete(globalName); } - return this.evaluator; + + for (String globalName : globalsValues.keySet()) + { + evaluator.set(globalName, globalsValues.get(globalName)); + } + } + + public Evaluator getEvaluator() + { + return evaluator; } public Lock getLock() diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java index 16f452df86deaec5ef8d1fa83a4f24d0b8d11b26..2eaf61bd265478ba1bef4aad32e3cddf663cceb6 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java @@ -107,7 +107,8 @@ public class JythonDynamicPropertyCalculator implements IDynamicPropertyCalculat try { evaluator.set(ENTITY_VARIABLE_NAME, entity); - return evaluator.evalAsStringLegacy2_2(); + String x = evaluator.evalAsStringLegacy2_2(); + return x; } finally { evaluator.releaseResources(); diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java index c4fca183e011087b613b9b65440c95b4487f2d8c..0ff99ad9fd6f1fc75e9686bed45b30c218f506f5 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java @@ -48,13 +48,16 @@ import org.jmock.Mockery; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import ch.systemsx.cisd.common.filesystem.FileUtilities; import ch.systemsx.cisd.common.jython.evaluator.Evaluator; +import ch.systemsx.cisd.common.utilities.TestResources; import ch.systemsx.cisd.openbis.generic.server.JythonEvaluatorPool.EvaluatorState; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IScriptDAO; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ScriptType; import ch.systemsx.cisd.openbis.generic.shared.dto.ScriptPE; import ch.systemsx.cisd.openbis.generic.shared.managed_property.IAtomicEvaluation; +import ch.systemsx.cisd.openbis.generic.shared.managed_property.IEvaluationRunner; /** * @author anttil @@ -78,6 +81,39 @@ public class JythonEvaluatorPoolTest } } + @Test + public void globalVariablesAreHandledCorrectlyWhenThereAreNestedCalls() throws Exception + { + TestResources resources = new TestResources(getClass()); + String script = FileUtilities.loadToString(resources.getResourceFile("recursive_property.py")); + final IEvaluationRunner runner = + pool.getRunner("calculate()", Math.class, script); + + final IAtomicEvaluation<String> action = new IAtomicEvaluation<String>() + { + + int counter = 5; + + @Override + public String evaluate(Evaluator evaluator) + { + evaluator.set("action", this); + evaluator.set("runner", runner); + evaluator.set("value", counter); + if (counter > 0) + { + counter--; + return evaluator.evalAsStringLegacy2_2(); + } else + { + return ""; + } + } + }; + runner.evaluate(action); + // Assertion in the python script! + } + @Test public void globalVariablesSetInPreviousRunsAreClearedBeforeNewEvaluation() throws Exception { diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTestResources/recursive_property.py b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTestResources/recursive_property.py new file mode 100644 index 0000000000000000000000000000000000000000..255a6cefeed749b24f8b3f12c9bfad93ff795761 --- /dev/null +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTestResources/recursive_property.py @@ -0,0 +1,5 @@ +def calculate(): + temp = value + runner.evaluate(action); + if temp != value: + raise Exception(str(temp)+" vs "+str(value)) \ No newline at end of file