enso-org / enso

Enso Analytics is a self-service data prep and analysis platform designed for data teams.
https://ensoanalytics.com
Apache License 2.0
7.36k stars 323 forks source link

State and Contexts are not preserved through the Polyglot boundary #7117

Open radeusgd opened 1 year ago

radeusgd commented 1 year ago

Whenever I call an Enso callback from within a polyglot call (i.e. I pass an Enso callback to a Java code as Function<A,B> and then execute that function in the Java code), the data about the State and more importantly Contexts is lost.

Repro

Create a new project, with a file polyglot/java/foo/foo/Foo.java:

package foo;

import java.util.function.Function;
import org.graalvm.polyglot.Value;

public class Foo {
    public static Value bar(Function<Integer, Value> f) {
        Value result = f.apply(42);
        return result;
    }
}

then in polyglot/java/foo run javac foo/Foo.java to get polyglot/java/foo/foo/Foo.class.

Now modify src/Main.enso to be:

from Standard.Base import all
import Standard.Base.Runtime.Context
import Standard.Base.Runtime.State

polyglot java import foo.Foo

run_inside_polyglot_callback ~action =
    callback _ = 
        action
    Foo.bar callback

report_states =
    IO.println "Context.Output.is_enabled = "+Context.Output.is_enabled.to_text
    state = Panic.catch Any (State.get Integer).to_text caught_panic->
        caught_panic.payload.to_display_text
    IO.println "State.get Integer = "+state

main =
    State.run Integer 123 <|
        IO.println "Direct:"
        report_states
        IO.println '\n'

        IO.println "Inside polyglot callback:"
        run_inside_polyglot_callback <| report_states
        IO.println '\n'

        IO.println '---------\n'
        IO.println "Now disabling context."
        Context.Output.with_disabled <|
            IO.println "Direct:"
            report_states
            IO.println '\n'

            IO.println "Inside polyglot callback:"
            run_inside_polyglot_callback <| report_states
            IO.println '\n'

and run the project.

Expected results

Direct:
Context.Output.is_enabled = True
State.get Integer = 123

Inside polyglot callback:
Context.Output.is_enabled = True
State.get Integer = 123

---------

Now disabling context.
Direct:
Context.Output.is_enabled = False
State.get Integer = 123

Inside polyglot callback:
Context.Output.is_enabled = False
State.get Integer = 123

Actual results

Direct:
Context.Output.is_enabled = True
State.get Integer = 123

Inside polyglot callback:
Context.Output.is_enabled = True
State.get Integer = State is not initialized for type Integer.

---------

Now disabling context.
Direct:
Context.Output.is_enabled = False
State.get Integer = 123

Inside polyglot callback:
Context.Output.is_enabled = True
State.get Integer = State is not initialized for type Integer.

We can see that inside of the callbacks that were executed by a Java call both the State and output context are lost - the state is treated as uninitialized and the Output context gets 're-enabled'.

I don't think this is correct.

radeusgd commented 1 year ago

I'm implementing some synchronization primitives in #7072 using Java and callbacks and then inside of such synchronized methods the execution context information gets reset, leading to invalid results in my computations.

I will add workarounds for this, but I think the current behaviour is incorrect and not intuitive.

If we have to keep it, we at least need to very carefully document this oddity somewhere.

JaroslavTulach commented 1 year ago

Currently State is passed as an argument throughout the Enso interpret execution. Such argument is obviously lost whenever a callback to and from polyglot call is made. To fix that we would need to keep state as thread local - either as a value in EnsoContext or as real ThreadLocal variable.

We should use ContextThreadLocal to store the State per EnsoContext and per thread.

radeusgd commented 3 months ago

Bumping this issue as I ran into it again when working on #10609 - I was surprised why a State read was not finding the value that I was sure is there, and I found out it was because of this bug.