Graaljs: allow multi thread access

[pmlopes]

JavaScript does not specify how it should behave in multi threading environments and for example "nashorn" would allow it, stating that the engine is thread safe but user code not. For certain environments (e.g.: vertx) that is not a big problem due to it's single thread/1 thread per cpu architecture.

So a script like:

vertx
  .createHttpServer()
  .requestHandler(function (req) {
    req.response().end("Hello!");
  })
  .listen(8080);

console.log('Server listening at: http://localhost:8080/');

Would behave the same as currently with nashorn. Sadly it's not the case see:

https://twitter.com/pml0pes/status/1006256574334107649

[thomaswue]

Can you please post the relevant parts of the outputs from that animated GIF? It is a bit hard to understand what is the issue. Generally, we do support sharing of JavaScript code between threads.

[pmlopes]

of course, the command run is:

~/.local/graalvm/bin/node --polyglot --jvm.classpath=./rt.jar
> require('@vertx/core/runtime');
{}
> require('./index.js');
Server listening at: http://localhost:8080/
{}

The file @vertx/core/runtime contains:

// reduced the script to the bare basic
global['vertx'] = Java.type('io.reactiverse.es4x.impl.graal.GraalJSRuntime').install({}, JSON, clustered);

Where the java code is:

// only the basic stuff here
return Vertx.vertx();

Then the main script is:

vertx
  .createHttpServer()
  .requestHandler(function (req) {
    req.response().end("Hello!");
  })
  .listen(8080);

when running with the node wrapper the output is:

> Unhandled exception java.lang.IllegalStateException: Multi threaded access requested by thread Thread[vert.x-eventloop-thread-1,5,main] but is not allowed for language(s) js.
    at com.oracle.truffle.api.vm.PolyglotContextImpl.throwDeniedThreadAccess(PolyglotContextImpl.java:566)
    at com.oracle.truffle.api.vm.PolyglotContextImpl.checkAllThreadAccesses(PolyglotContextImpl.java:484)
    at com.oracle.truffle.api.vm.PolyglotContextImpl.enterThreadChanged(PolyglotContextImpl.java:405)
    at com.oracle.truffle.api.vm.PolyglotContextImpl.enter(PolyglotContextImpl.java:365)
    at com.oracle.truffle.api.vm.PolyglotBoundaryRootNode.execute(PolyglotBoundaryRootNode.java:69)
    at org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callProxy(OptimizedCallTarget.java:262)
    at org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callRoot(OptimizedCallTarget.java:251)
    at org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callBoundary(OptimizedCallTarget.java:241)
    at org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.doInvoke(OptimizedCallTarget.java:226)
    at org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.call(OptimizedCallTarget.java:202)
    at com.oracle.truffle.api.interop.java.FunctionProxyHandler.invoke(JavaInteropReflect.java:427)
    at com.sun.proxy.$Proxy35.handle(Unknown Source)
    at io.vertx.core.http.impl.Http1xServerConnection.processMessage(Http1xServerConnection.java:433)
    at io.vertx.core.http.impl.Http1xServerConnection.handleMessage(Http1xServerConnection.java:141)
    at io.vertx.core.http.impl.HttpServerImpl$ServerHandlerWithWebSockets.handleMessage(HttpServerImpl.java:683)
    at io.vertx.core.http.impl.HttpServerImpl$ServerHandlerWithWebSockets.handleMessage(HttpServerImpl.java:636)
    at io.vertx.core.net.impl.VertxHandler.lambda$channelRead$1(VertxHandler.java:146)
    at io.vertx.core.impl.ContextImpl.lambda$wrapTask$2(ContextImpl.java:337)
    at io.vertx.core.impl.ContextImpl.executeFromIO(ContextImpl.java:195)
    at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:144)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
    at io.vertx.core.http.impl.HttpServerImpl$Http2UpgradeHandler.channelRead(HttpServerImpl.java:952)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
    at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:310)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:284)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
    at io.vertx.core.http.impl.Http1xOrH2CHandler.end(Http1xOrH2CHandler.java:60)
    at io.vertx.core.http.impl.Http1xOrH2CHandler.channelRead(Http1xOrH2CHandler.java:38)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1359)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:935)
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:141)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:645)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:580)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:497)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:459)
    at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:886)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.lang.Thread.run(Thread.java:748)

However if I run the script in embedded mode it works:

~/.local/graalvm/bin/java -jar rt.jar 
Server listening at: http://localhost:8080/
Succeeded in deploying verticle 

And the curl command completes successfully.

[pmlopes]

This code uses many SNAPSHOT versions so if you need a working example I can push a snapshot to sonatype oss and make a gist with the example app.

[wirthi]

Hi Paulo,

thanks for reporting this. Without looking at your code, I assume your first sentences sum up the problem nicely: we decided to forbid that pattern by default because it is insecure.

If that is the actual problem, I am sure we can provide a solution for you on this. I will discuss that with our engineers and get back to you.

Thanks,
Christian

[pmlopes]

Yes I am aware of the security problems but in vertx case we can always rely on the JVM primitive synchronized or use the java.util.Concurrent classes. Perhaps a flag to disable it could do the trick?

[thomaswue]

That would work. But maybe we can have a better solution that satisfies your use case and provides also better safety. Can we setup a call to discuss?

[woess]

For certain environments (e.g.: vertx) that is not a big problem due to it's single thread/1 thread per cpu architecture.

Can you elaborate a bit more on that? It does not sound like you'd only ever run callback functions from one thread at a time.

The main reason why we currently don't allow objects/functions be used by multiple threads is that JS objects are not thread-safe and you need to explicit synchronization to make it safe. We can probably relax this restriction where we know it's safe to do so.

[pmlopes]

@woess the best document we have explaining the details is here: https://github.com/vietj/vertx-materials/blob/master/src/main/asciidoc/output/Demystifying_the_event_loop.adoc

So in a nutshell, each EventLoop is a Thread that scheldules callbacks for execution, this means that at a given time only a callback is executing a script function, once a callback is executed in a event loop, there is a context, which we then ensure that further callbacks will always execute on the same event loop to avoid context switching.

Now the mt issues can happen when there are many event loops (multiple cores) in that case we advise users who need to share state not to use global variables but use a special data structure (sharedData) that has builtin synchronization.

If a user does want to use global variables, we don't forbid but is synchronization issues happen it's a fact that users need to address themselves. (say use locks, latches, etc...)

[chumer]

I think i found a solution that allows to transfer control to another thread while the current thread is still active.

Unfortunately it exposed a bug in RC2 that we will fix. Namely we enter again for eval calls also if it was already entered explicitely. If the context was entered explicitely we can also leave it explicitly while the JS engine is running. I also need to do some more testing such that this pattern cannot be misused. For example that we fail properly if you don't reenter the context when the event queue completes.

Here is my solution that uses a simple Thread that should indicate the event loop. Let me know if this captures your problem appropiately.

 public static class HttpServer {

        private final Vertx vertx;
        private final List<Value> requestHandlers = new ArrayList<>();

        HttpServer(Vertx vertx) {
            this.vertx = vertx;
        }

        public HttpServer requestHandler(Value v) {
            requestHandlers.add(v);
            return this;
        }

        public void listen(int port) {
            vertx.context.leave();
            Thread thread = new Thread(new Runnable() {
                @Override
                public void run() {
                    vertx.context.enter();
                    for (Value handler : requestHandlers) {
                        handler.execute();
                    }
                    vertx.context.leave();
                }
            });
            thread.start();
            try {
                thread.join();
            } catch (InterruptedException e) {
            }
            vertx.context.enter();
        }
    }

    public static class Vertx {

        private final Context context;

        Vertx(Context context) {
            this.context = context;
        }

        public HttpServer createHttpServer() {
            return new HttpServer(this);
        }

    }

    public static void main(String[] args) {
        Context context = Context.create();
        context.getBindings("js").putMember("vertx", new Vertx(context));
        context.enter();

        // workaround
        context.eval("js", "function() {" +
                        "  vertx\n" +
                        "  .createHttpServer()\n" +
                        "  .requestHandler(function () {\n" +
                        "    console.log('handler');\n" +
                        "  })\n" +
                        "  .listen(8080);\n" +
                        "\n" +
                        "console.log('Server listening at: http://localhost:8080/');}").execute();

        // broken in RC2 (will fix for RC3)
        context.eval("js", "vertx\n" +
                        "  .createHttpServer()\n" +
                        "  .requestHandler(function () {\n" +
                        "    console.log('handler');\n" +
                        "  })\n" +
                        "  .listen(8080);\n" +
                        "\n" +
                        "console.log('Server listening at: http://localhost:8080/');");
        context.leave();
        context.close();
    }

[pmlopes]

2 comments:

I'm not sure if this will work when using the node wrapper as there environment is already set and there is no way to access the context.

Does this mean that we need to guard all received Value objects at the java side with:

try {
  context.enter();
  value.execute();
} finally {
  context.leave();
}

This would be a very explicit way to do it and "all" libraries would have to have this in mind, so we could not use any arbitrary java library I guess, unless something like: https://github.com/oracle/graal/issues/464 is in place and we can add an interceptor, but still it would be quite hard to get it right as the target could be anything, perhaps filter on source being a function... but I might be missing something here.

[chumer]

I'm not sure if this will work when using the node wrapper as there environment is already set and there is no way to access the context.

I forgot about the node wrapper requirement. Right it does not work in that environment, because node has control over the context. Let me think about this more.

Does this mean that we need to guard all received Value objects at the java side with:

If you don't do it explicitely the value would enter for you. But if you don't do it explicitely you cannot explicitely leave later on.

[chumer]

After discussing with @pmlopes I think we agreed that that the current solution is sufficient.
Here is the fix to get rid of the workaround: https://github.com/oracle/graal/commit/5214ee1a70c764bf7a8dc074ed6a4df31c2db095

Closing this as fixed. Feel free to reopen if there are further questions.

[hanzr]

I think i found a solution that allows to transfer control to another thread while the current thread is still active.

Christian, in your example the main thread is not really active, is it? It starts the handler thread and waits for that to finish, in thread.join()..