Hi Joao,

please see below.

Am 20.02.2017 um 14:50 schrieb João Paulo Varandas 
<joaovaran...@inpaas.com<mailto:joaovaran...@inpaas.com>>:

There is no issue there.
When your code is evaluated, currentThreadName is not a pointer to current 
thread name. It is a string, that has been evaluated only once.

It won't change in different executions, remaining as "main" despite the 
executing thread(the closure has been created and it's variables are safely 
stored inside the closure).

You are right, this is by design of Javascript closures, and there is no issue 
with Nashorn here (there may be an issue with my Javascript knowledge, though 
;-)

In your fix for the closure, you replaced the String object with a function 
object (that calls Thread.currentThread()). The test will fail equally if you 
move the brackets „()“ of the function invocation from the innerFunction into 
the last line of the JS, so that we’ll have a Thread object in the closure, see 
https://gist.github.com/jfrantzius/0a40c963413bdeabb51ecb13a769a436#file-nashornclosuretest-java-L45

It’s a nice fix if you have control of the Javascript code. Unfortunately, 
we’re using an existing Javascript library that wasn’t designed with 
multi-threading in mind, and we cannot change the code.

So my point maybe then is that crosstalk issues with existing Javascript code 
may well be related to closures in that code. And I can also imagine that 
closures may cause headaches with someone’s own code, if it's supposed to be 
reentrant...

Regards,
Jörg


Check out the changes in the code:

  @Test
    public void testClosureThreadSafety() throws ScriptException {
        String testJsFunction = (
            "    (function outerFunction(currentThreadName) {\n" +
            "          function innerFunction() {\n" +
            "            print(currentThreadName, 
java.lang.Thread.currentThread().toString()); return currentThreadName;\n" +
            "          }\n" +
            "          return innerFunction;\n" +
            "    })(java.lang.Thread.currentThread().toString())\n");

        ScriptObjectMirror jsFunction = (ScriptObjectMirror) 
e.eval(testJsFunction);

        String currentThreadName = Thread.currentThread().toString();

        IntConsumer invokeAndTest = i-> {
            Object received = jsFunction.call(jsFunction);

            Assert.assertEquals(currentThreadName, received);
        };
        IntStream.range(0, 10).parallel().forEach(invokeAndTest);
    }

I've added a "print(currentThreadName, 
java.lang.Thread.currentThread().toString()); " to prove that currentThreadName 
stays the same in all executions despite which Thread it is executing.


Check out this one now:

  @Test
    public void testClosureThreadSafety2() throws ScriptException {
        String testJsFunction = (
            "    (function outerFunction(threadPointer) {\n" +
            "          function innerFunction() {\n" +
            "            return threadPointer().toString();\n" +
            "          }\n" +
            "          return innerFunction;\n" +
            "    })(java.lang.Thread.currentThread)\n");

        ScriptObjectMirror jsFunction = (ScriptObjectMirror) 
e.eval(testJsFunction);

        IntConsumer invokeAndTest = i-> {
            Object received = jsFunction.call(jsFunction);

            Assert.assertEquals(Thread.currentThread().toString(), received);
        };
        IntStream.range(0, 10).parallel().forEach(invokeAndTest);
    }

Now there's a pointer to currentThread(not a string), and you can use it in 
every thread returning different data.








[https://static.inpaas.com/assets/inpaas/images/logo-img-50px.png]
João Varandas
Arquiteto de Soluções Cloud
inPaaS - Idéias em Aplicações


p:      +55 11 5091-2777  m: +55 11 99889-2321
a:      Rua Nebraska, 443 - 1o Andar
        Brooklin Paulista, São Paulo, SP
w:      www.inpaas.com<http://www.inpaas.com/>  e: 
joaovaran...@inpaas.com<mailto:joaovaran...@inpaas.com>



2017-02-20 10:32 GMT-03:00 Frantzius, Jörg 
<joerg.frantz...@aperto.com<mailto:joerg.frantz...@aperto.com>>:
Hi Joao,

the following test fails immediately for me with "java.lang.RuntimeException: 
Expected: Thread[ForkJoinPool.commonPool-worker-4,5,main], received: 
Thread[main,5,main]":

   @Test
    public void testClosureThreadSafety() throws ScriptException {
        final ScriptEngine engine = new 
ScriptEngineManager().getEngineByName("nashorn");

        String testJsFunction = (
            "    (function outerFunction(currentThreadName) {\n" +
            "          function innerFunction() {\n" +
            "            return currentThreadName;\n" +
            "          }\n" +
            "          return innerFunction;\n" +
            "    })(java.lang.Thread.currentThread().toString())\n");

        ScriptObjectMirror jsFunction = (ScriptObjectMirror) 
engine.eval(testJsFunction);

        IntConsumer invokeAndTest = i-> {
            String currentThreadName = Thread.currentThread().toString();
            Object received = jsFunction.call(jsFunction);
            if (!currentThreadName.equals(received)) {
                throw new RuntimeException("Expected: " + currentThreadName + 
", received: " + received);
            }
        };
        IntStream.range(0, 10).parallel().forEach(invokeAndTest);
    }

The outer function returns its inner function, which contains „currentThread“ 
as a reference to its closure (i.e. a reference to outerFunction’s 
„currentThread“ parameter). That closure property „currentThread“ will be set 
to the name of the current thread only once (in engine.eval(testJsFunction)), 
and subsequent calls to innerFunction will always return the name of that 
thread (and not of the current thread that calls innerFunction).

If your Javascript code is under your control, this may not be a problem, as 
you can change the code. In our case, we are using an existing Javascript 
library „Handlebars“ that we cannot change, which seems to be keeping function 
objects with closures around just like the above code does in Java.

Regards,
Jörg

Am 20.02.2017 um 13:00 schrieb João Paulo Varandas 
<joaovaran...@inpaas.com<mailto:joaovaran...@inpaas.com>>:

Hi Jorg.

Could you send us a code snippet?

I have never seem such problem when using closures. In my project, I use a 
single engine for whole web application. My tomcat is running with 150 
maxThreads and it seems to be working fine. I test that in each build by 
running the test case below:

https://gist.github.com/joaovarandas/f80a9cb5548a9d620e4da1ace2729911

The idea in this test is to use a single engine and run a closure from 
one-thread or multiple-threads simultaneously and then read data from those 
closures.


PS.: Should I send the source code directly in the mail body for future readers?





[https://static.inpaas.com/assets/inpaas/images/logo-img-50px.png]
João Varandas
Arquiteto de Soluções Cloud
inPaaS - Idéias em Aplicações


p:      +55 11 5091-2777<tel:+55%2011%205091-2777>  m: +55 11 
99889-2321<tel:+55%2011%2099889-2321>
a:      Rua Nebraska, 443 - 1o Andar
        Brooklin Paulista, São Paulo, SP
w:      www.inpaas.com<http://www.inpaas.com/>  e: 
joaovaran...@inpaas.com<mailto:joaovaran...@inpaas.com>



2017-02-19 18:30 GMT-03:00 Frantzius, Jörg 
<joerg.frantz...@aperto.com<mailto:joerg.frantz...@aperto.com>>:
… to correct myself, with code that contains closures, it’s probably 
global-per-thread on a single engine that remains as the least 
resource-consuming option (we were using a single global on single engine for 
all threads, in order to share expensively computed Javascript state between 
them).

From what I understand, global-per-thread could be implemented e.g. by having a 
ThreadLocal<ScriptContext> and always using that as the context in 
ScriptEngine.eval(script, context).

It would be good to know then whether global-per-thread on single engine still 
allows for sharing Nashorn’s code optimization between threads? That would 
already be great (and as Nashorn *is* great, I’m positive here :)

Regards,
Jörg


Am 19.02.2017 um 00:47 schrieb Frantzius, Jörg 
<joerg.frantz...@aperto.com<mailto:joerg.frantz...@aperto.com><mailto:joerg.frantz...@aperto.com<mailto:joerg.frantz...@aperto.com>>>:

Hi,

it begins to dawn on me that closures aren’t thread-safe, at least that would 
explain crosstalk issues we’re seeing in JMeter tests (with a single engine for 
multiple threads).

It would be good to know (and I guess for others as well) if somebody can 
confirm this?

Perhaps thread-safety of closures was thinkable if Nashorn somehow stored 
closure state in ThreadLocals, but I guess that’s neither happening nor planned?

From what I understand, closures are pervasive in Javascript code out there, 
and anybody using such code will currently be forced to use engine-per-thread.

Thanks for any hints,
Jörg


---

Dipl. Inf. Jörg von Frantzius, Technical Director

E-Mail 
joerg.frantz...@aperto.com<mailto:joerg.frantz...@aperto.com><mailto:joerg.frantz...@aperto.com<mailto:joerg.frantz...@aperto.com>>

Phone +49 30 283921-318<tel:%2B49%2030%20283921-318>
Fax +49 30 283921-29<tel:%2B49%2030%20283921-29>

Aperto GmbH – An IBM Company
Chausseestraße 5, D-10115 Berlin
http://www.aperto.com<http://www.aperto.com/><http://www.aperto.de/>
http://www.facebook.com/aperto
https://www.xing.com/companies/apertoag

HRB 77049 B, AG Berlin Charlottenburg
Geschäftsführer: Dirk Buddensiek, Kai Großmann, Stephan Haagen, Daniel Simon



---

Dipl. Inf. Jörg von Frantzius, Technical Director

E-Mail joerg.frantz...@aperto.com<mailto:joerg.frantz...@aperto.com>

Phone +49 30 283921-318<tel:%2B49%2030%20283921-318>
Fax +49 30 283921-29<tel:%2B49%2030%20283921-29>

Aperto GmbH – An IBM Company
Chausseestraße 5, D-10115 Berlin
http://www.aperto.com<http://www.aperto.com/><http://www.aperto.de/>
http://www.facebook.com/aperto
https://www.xing.com/companies/apertoag

HRB 77049 B, AG Berlin Charlottenburg
Geschäftsführer: Dirk Buddensiek, Kai Großmann, Stephan Haagen, Daniel Simon



"Esta mensagem, incluindo seus anexos, pode conter informacoes confidenciais e 
privilegiadas.
Se voce a recebeu por engano, solicitamos que a apague e avise o remetente 
imediatamente.
Opinioes ou informacoes aqui contidas nao refletem necessariamente a posicao 
oficial da Plusoft."

"Antes de imprimir, pense em sua responsabilidade e compromisso com o MEIO 
AMBIENTE"



---

Dipl. Inf. Jörg von Frantzius, Technical Director

E-Mail joerg.frantz...@aperto.com

Phone +49 30 283921-318<tel:+49%2030%20283921318>
Fax +49 30 283921-29<tel:+49%2030%2028392129>

Aperto GmbH – An IBM Company
Chausseestraße 5, D-10115 Berlin
http://www.aperto.com<http://www.aperto.de/>
http://www.facebook.com/aperto
https://www.xing.com/companies/apertoag

HRB 77049 B, AG Berlin Charlottenburg
Geschäftsführer: Dirk Buddensiek, Kai Großmann, Stephan Haagen, Daniel Simon



"Esta mensagem, incluindo seus anexos, pode conter informacoes confidenciais e 
privilegiadas.
Se voce a recebeu por engano, solicitamos que a apague e avise o remetente 
imediatamente.
Opinioes ou informacoes aqui contidas nao refletem necessariamente a posicao 
oficial da Plusoft."

"Antes de imprimir, pense em sua responsabilidade e compromisso com o MEIO 
AMBIENTE"



---

Dipl. Inf. Jörg von Frantzius, Technical Director

E-Mail joerg.frantz...@aperto.com

Phone +49 30 283921-318
Fax +49 30 283921-29

Aperto GmbH – An IBM Company
Chausseestraße 5, D-10115 Berlin
http://www.aperto.com<http://www.aperto.de/>
http://www.facebook.com/aperto
https://www.xing.com/companies/apertoag

HRB 77049 B, AG Berlin Charlottenburg
Geschäftsführer: Dirk Buddensiek, Kai Großmann, Stephan Haagen, Daniel Simon

Reply via email to