Rerunning ScriptModule fails

[Squareys]

Hello everybody!

I have a use case in which I would like to reuse a ScriptModule, but the Jython script always returns the same value (the value of the first execution). Consider the following test (you may copy into the JythonTest and autoimport to run):

    @Test
    public void testMultipleExecution() throws InterruptedException, ExecutionException,
        IOException, ScriptException, ModuleException
    {
        final Context context = new Context(ScriptService.class);
        final ModuleService moduleService = context.getService(ModuleService.class);

        final String TEST_CODE = "" + //
            "# @INPUT String str\n" + //
            "# @OUTPUT String out\n" +  //
            "out = \"Hello, \" + str\n";

        ScriptInfo info = new ScriptInfo(context, "hello.py", new StringReader(TEST_CODE));
        ScriptModule module = info.createModule();

        context.inject(module);

        module.setInput("str", "Albert");
        module.setResolved("str", true);
        moduleService.run(module, true).get();
        assertEquals("Hello, Albert", module.getOutput("out"));

        module.setInput("str", "Einstein");
        moduleService.run(module, true).get();

        assertEquals("Hello, Einstein", module.getOutput("out"));
    }

which gives the following output:

org.junit.ComparisonFailure: expected:<Hello, [Einstein]> but was:<Hello, [Albert]>

Debugging further this problem seems to go all the way down to everything working as expected until this line, where the out var from the script is not replaced with the new value.

Anybody have an idea? Or is there a different way of rerunning a script efficiently instead of reusing a module?

Thanks in advance!
Jonathan

[ctrueden]

Module instances are not intended to be reused. Create a new one each time from the ModuleInfo.

[ctrueden]

That said: the behavior you describe is pretty arcane, and could probably be considered a bug. A clearer error message would be nice.

[ctrueden]

Off the top of my head, the cause is probably that the module inputs and outputs get marked as resolved after the first execution, and would need to be marked un-resolved again in order for it to run again.

[Squareys]

Module instances are not intended to be reused. Create a new one each time from the ModuleInfo.

Alright, I kinda expected that. The inputs obviously don't need to be unresolved, since I am setting them again anyway, which resolves them. Not so obviously, the resolved state of the output does not matter for the above mentioned line either, since it is not tested before the module is run (before setting the ouput, it does check for resolved, though).

Anyhow, a little more surprising, recreating the Module from the ScriptInfo does not work either:

    @Test
    public void testMultipleExecution() throws InterruptedException, ExecutionException,
        IOException, ScriptException, ModuleException
    {
        final Context context = new Context(ScriptService.class);
        final ModuleService moduleService = context.getService(ModuleService.class);

        final String TEST_CODE = "" + //
            "# @INPUT String str\n" + //
            "# @OUTPUT String out\n" +  //
            "out = \"Hello, \" + str\n";

        // first execution
        ScriptInfo info = new ScriptInfo(context, "hello.py", new StringReader(TEST_CODE));
        ScriptModule module = info.createModule();
        context.inject(module);

        module.setInput("str", "Albert");
        module.setResolved("str", true);
        moduleService.run(module, true).get();
        assertEquals("Hello, Albert", module.getOutput("out"));

        // second execution
        module = info.createModule();
        context.inject(module);
        module.setInput("str", "Einstein");
        module.setResolved("str", true); // <-- [EDIT]
        moduleService.run(module, true).get();

        assertEquals("Hello, Einstein", module.getOutput("out"));
    }

... which results in:

java.lang.AssertionError: expected:<Hello, Einstein> but was:<null>

[Squareys]

Alright, I quickly debugged into this, and found the bug: the Reader for the Script is not reset after the Script has been run, so the next time we run a script, it simply "starts parsing" from the end, where it stopped last.
This is actually a bug in scijava-common, which means multiple execution could be broken for all scripting languages, seems as if nobody needed this yet.

I'm not completely sure what the cleanest way to go about this would be, but I'll try to make something up and file a PR ;)

[Squareys]

I added a line to the above code (see the // <-- [EDIT] comment), which I had originally forgotten. It turns out, that without that line, the first module will leak its input parameters to the second module, which also results in unexpeced behaviour.

I believe the ScriptModule should cleanup the variables in the ScriptEngine after it is done executing. Can this be safely done for all languages? We should at least check if the input parameters were actually resolved, before setting them in the engine bindings.

[Squareys]

TODOs to close this issue:

• Merge scijava/scijava-common#194
• Check input variables in ScriptModule before settings the engine bindings for them
• Cleanup ScriptEngine bindings after ScriptModule has run (should be discussed first)

[ctrueden]

@Squareys Can this issue be closed now?