Problem with multi thread access on GraalJS

[Brunomachadob]

Hey! Thank you guys for such a great work on GraalJS, I'm really enjoying working with it.

I'm currently working in a development platform called ThrustJS (sorry about the docs that are only in Portuguese by now) in my company, it's written in JavaScript, powered by the JVM using Nashorn. We already have a lot of projects running in this platform.

Recently I started to work on this platform to run on both Nashorn and GraalJS, everything is going well so far, except by this error I'm getting when I try to use the Apache Tomcat 9 as an API server.

java.lang.IllegalStateException: Multi-threaded access requested by thread Thread[http-nio-8778-exec-1,5,main] but is not allowed for language(s) js.
	com.oracle.truffle.polyglot.PolyglotContextImpl.throwDeniedThreadAccess(PolyglotContextImpl.java:545)
	com.oracle.truffle.polyglot.PolyglotContextImpl.checkAllThreadAccesses(PolyglotContextImpl.java:464)
	com.oracle.truffle.polyglot.PolyglotContextImpl.enterThreadChanged(PolyglotContextImpl.java:383)
	com.oracle.truffle.polyglot.PolyglotContextImpl.enter(PolyglotContextImpl.java:344)
	com.oracle.truffle.polyglot.PolyglotValue$PolyglotNode.execute(PolyglotValue.java:512)
	org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callProxy(OptimizedCallTarget.java:269)
	org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callRoot(OptimizedCallTarget.java:258)
	org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callBoundary(OptimizedCallTarget.java:248)
	org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.doInvoke(OptimizedCallTarget.java:230)
	org.graalvm.compiler.truffle.runtime.GraalTVMCI.callProfiled(GraalTVMCI.java:86)
	com.oracle.truffle.api.impl.Accessor.callProfiled(Accessor.java:702)
	com.oracle.truffle.polyglot.VMAccessor.callProfiled(VMAccessor.java:75)
	com.oracle.truffle.polyglot.PolyglotValue$Interop.executeVoid(PolyglotValue.java:1370)
	org.graalvm.polyglot.Value.executeVoid(Value.java:333)
	javax.servlet.http.HttpServlet$$JSJavaAdapter.service(Unknown Source)

Today we don't have any sync/shared state problems as we hardly encourage everyone to use more functional/stateless approaches to build applications and also, we always create a new script context on every require on a file. (See creating a new context on require)

I have isolated the problem in a simple Gist, the example works using jjs but doesn't work with 'js'.

I did some digging in the GraalJS/GraalVM codebase and issues and it seems we don't have any kind of option to disable this.

Could you guys please give me some advice on this?

Thank's in advance.

[eleinadani]

Hi @Brunomachadob,

thanks for your interest in Graal.js.

The code in your example attempts to access the same JS Context from multiple threads, concurrently. Concurrent access to the same JS Context is not allowed in Graal.js, which strictly forbids multi-threaded concurrent access. More precisely, Tomcat attempts to execute this function (which is created by the main JS thread) in one of its worker threads. The original JS Context where the function was created is however still active (at tomcat.getServer().await()), thus two threads are found in the same active JS Context (hence, you get the exeception).

This is conceptually equivalent to the scenario in this test. Graal.js does allow multi-threading, but the application must ensure (e.g., using thread locals or synchronization to enter the right Context) that a JS Context is not accessed by two threads at the same time. Here you can find some other examples of multi-threaded Context usages.

[Brunomachadob]

Thanks for your quick response @eleinadani.

Yes, I understood the problem regarding the concurrence of the two threads that were active at the same time.

What I'm struggling to understand is how I could avoid this problem, considering that:

• The main thread is started by js
• The service threads are managed entirely by Tomcat.

Looking on your tests, you have complete control of the execution cycle, your created threads and the JS Context, which is not my case.
Or, maybe I'm wrong about this statement, in this case, could you help me to clarify this?

Where I could create/enter/leave Contexts in the provided example?

Thanks!

[eleinadani]

Currently, creating, entering and leaving another JS context from JavaScript is not supported. As discussed in our documentation, you can however use a Java class (e.g., Runnable, or HttpServlet in this case), and enter/leave a different JS context from there.

For example, you could define:

public class MyServlet extends HttpServlet {
	
	private final Value handler;
	private final Context cx;

	public MyServlet(String src) {
		this.cx = Context.create();
		this.handler = cx.eval("js", src);
	}
	
	@Override
	protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
		try {
			cx.enter();
			handler.execute(request, response);
		} finally {
			cx.leave();
		}
	}	
}

and create the servlet from your JS code in the following way:

var ServletClass = Java.type('MyServlet');

/* ... */

function init() {

    /* ... */

    Tomcat.addServlet(ctx, "myServlet", new ServletClass(`
            function (request, response) {
               response.setStatus(200)
               response.setContentType('text/html; charset=utf-8')
               response.getWriter().println('MySimpleServletResponse')
               response.flushBuffer()
             }
        `))
}

Since it is not possible to share JS objects between contexts, the JS callback has to be passed to the MyServlet constructor as a string. Alternatively, you can put your JS code in a new file, and cx.eval() the entire file from MyServlet.

[Brunomachadob]

Thanks for your insights @eleinadani

This kind of change is going to demand a lot of rework, in a lot of libraries.

Also, this will complicate things, as today we only develop in JavaScript, with this kind of change will need to start doing some Java and building jars to use as dependencies.

Assuming that only our framework use this kind of usage of threads.
If any application also uses this kind of multi-threaded processing, then we have another place to rewrite.

Here we have another example scheduler that would need some work too.

And another, but this isn't ours: httpsrv.js

Do you guys have any plans to include some kind of option to disable this validation?
We could leave this option disabled by default, or, turn it on when using the Nashorn compatibility mode.

I'm sure this is going to break a lot of thinks that was written for Nashorn.

Let's say we simply remove that validation, my applications that today, is sharing contexts between threads using Nashorn will behave quite differently in GraalJS?

edited
Another point is, as I said before, we were trying to maintain things compatible with Nashorn and GraalJS, to migrate our platform and all of our application in the smoothest way possible.

GraalJS had some kind of change in JavaScript execution that made this validation needed or it was just a desired feature to include, to help minimize concurrency errors?

Thanks!

[eleinadani]

Yes, the main reason why we do not allow two concurrent threads to access the same Context at the same time is thread safety. In Nashorn, you could share any object between threads (e.g., the global object) leading to unexpected data races. We don't have concrete plans to support concurrent access to the same context at the moment, but we are investigating alternative solutions that might allow multi-threaded JS code (like in your examples) to run on Graal.js. We cannot estimate yet when such support will land.

[tosehee75]

Thanks @eleinadani.

Similar question, but slightly different. Is it possible to share the same javascript functions, but execute with different variables through Bindings or Values?

For example, I have 5 orders that need to be evaluated through the same source, but I don't want to reload and re-compile the script. I want to compile/prepare the sources, but execute them with different Order object.

Like you said, I am running into thread race issue with Nashorn, and looking for alternative and ran into this thread.

Thanks for your response in advance..

Cheers

[eleinadani]

We recently published this article where we summarize of the main aspects of Graal.js' threading support. I am therefore closing this issue, but feel free to re-open if you have further questions.

[iitsoftware]

I have to disagree with your strict threading policy. Like in Nashorn, it should be the responsibility of the user how to handle multi-threaded access.

Here is an example:

We have a MQTT connection and subscribe on a topic as a consumer. This is done asynchronously with a listener that gets called when a message arrives. That's a quite common pattern in messaging. Everything is initiated from the JS side. In my script I register the listener and a callback when a message arrives:

    var LISTENER = Java.extend(Java.type("org.eclipse.paho.client.mqttv3.IMqttMessageListener"),   {
        messageArrived: function(topic, message) {
            stream.executeCallback(function(msg) {
                self.executeOutputLink("Out", msg);
            }, message);
        }
    });
    this.client.subscribe(this.props["topicfilter"], qos(this.props["qos"]), new LISTENER());

The listener is called from the MQTT client thread. I have no access to it. I can't synchronize access to the context because I'm within the script. So this listener is called and I only use it to register another callback stream.executeCallback which calls it from an event thread to ensure the single-threaded access to the script. But the initial call to the listener is out of my control.

This works in Nashorn, of course. That it might turn into race conditions is true but to avoid that is the responsibility of the user, not of the JVM.

[hjerabek]

@iitsoftware: I strongly agree with your statement that this should be the responsibility of the developer. Did you manage to find a solution for that problem? While it is possible to start new threads with Java-implemented Runnables, I am curious if you happened to find a "JavaScript-only" solution...

[iitsoftware]

Yes, I did: https://medium.com/swlh/porting-from-nashorn-how-to-handle-multi-threading-in-graal-js-957e359b7df5 Am 01.08.2020 um 18:58 schrieb hjerabek <[email protected]>:  @iitsoftware: I strongly agree with your statement that this should be the responsibility of the developer. Did you manage to find a solution for that problem? While it is possible to start new threads with Java-implemented Runnables, I am curious if you happened to find a "JavaScript-only" solution... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[hjerabek]

@iitsoftware: Thanks for the link and sharing your code and experiences. Using a custom Proxy class is a clever and elegant solution. Yet, IMO there is no equivalent, JavaScript-only solution of this approach. If I, for example, use Java.extend on java.lang.reflect.Proxy to create a custom Proxy class, it should result in the original IllegalStateException because the functions used to extend Proxy are bound to the context they are defined in. Would you agree that probably any workaround for this problem will require at least some component to be coded in Java?

[iitsoftware]

Yes, you‘d at least need a custom Proxy class that instantiates the original Java interface and binds it to the Proxy. So this needs to be part of your runtime somehow. Find the code here: https://github.com/iitsoftware/graaljs-concurrency-problem Am 01.08.2020 um 20:54 schrieb hjerabek <[email protected]>:  @iitsoftware: Thanks for the link and sharing your code and experiences. Using a custom Proxy class is a clever and elegant solution. Yet, IMO there is no equivalent, JavaScript-only solution of this approach. If I, for example, use Java.extend on java.lang.reflect.Proxy to create a custom Proxy class, it should result in the original IllegalStateException because the functions used to extend Proxy are bound to the context they are defined in. Would you agree that probably any workaround for this problem will require at least some component to be coded in Java? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[guygoldwasser]

I wonder how can this being resolve using JSR223.
Recreation of invokable with different new Bindings not do the trick.
will be glad for a hint.
e.g. engine.getInterface(engine.eval("MyClass", bindings), MyClass.class);
and this interface should be use from different sessions. how can this being created under same engine instance without getting into this multi thread issue.
Thanks

[iitsoftware]

Look here: https://medium.com/swlh/porting-from-nashorn-how-to-handle-multi-threading-in-graal-js-957e359b7df5 <https://medium.com/swlh/porting-from-nashorn-how-to-handle-multi-threading-in-graal-js-957e359b7df5> Am 06.01.2022 um 14:28 schrieb guygoldwasser ***@***.***>:  I wonder how can this being resolve using JSR223. Recreation of invokable with different new Bindings not do the trick. will be glad for a hint. e.g. engine.getInterface(engine.eval("MyClass", bindings), MyClass.class); and this interface should be use from different sessions. how can this being created under same engine instance without getting into this multi thread issue. Thanks — Reply to this email directly, view it on GitHub <#59 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOQJLX2LIBPWBA2BRNGH6LUUWKG3ANCNFSM4FVQC7JA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&amp;mt=8&amp;pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&amp;referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub> . You are receiving this because you were mentioned. <https://github.com/notifications/beacon/AAOQJLWJMRV7UMEIEGUBOSTUUWKG3A5CNFSM4FVQC7JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHP7WC7Y.gif> Message ID: ***@***.***>

[caoccao]

I happened to notice this issue and am surprised by the solution based on AOP. In my humble opinion, thread-synchronizing belongs to the JS engine.

Here's another solution without locks in Java for your reference, though I know it makes little sense to folks who are locked in GraalJS. At least, in my solution, the JS engine does that work transparently.

[guygoldwasser]

Thanks @iitsoftware, will try that.
I agree with @caoccao and totally not understand why this issue is closed.
using Graal via JSR223 means not only wrap up all my interfaces, but also wrap with some proxy factory the abstract javax.script.CompiledScript class cause its 'eval' method is not an interface.

All together comes to conclusion of incompatibility to JSR223.

I tried the follow:

1. ThreadLocal to have Context per Thread like @eleinadani mention.
2. wrap all interface with Proxy, enter context before invoke and leave after: getContext() taking context from thread local.

public static <T> T wrap(final T obj, Class<?>... interfaces) {
  Class<? extends Object> cl = obj.getClass();
  InvocationHandler handler = new InvocationHandler() {
	  @Override
	  public Object invoke(Object o, Method method, Object[] args)
			  throws Throwable {
		  getContext().enter();
		  try {
			  return method.invoke(obj, args);
		  } finally {
			  getContext().leave();
		  }
	  }
  };
  return (T) Proxy.newProxyInstance(cl.getClassLoader(), interfaces, handler);
}

3. delegate all my CompiledScript 'eval's to this: getContext() same get it from thread local.

public final Object eval(CompiledScript script, Bindings bindings) throws ScriptException{
  Context context = ScriptEngineLoader.getContext();
  try{
     context.enter();
     return script.eval(bindings);
  }finally{
     context.leave();
  }
}

Still in same position with same exception.
Any idea what am I doing wrong?