Closures not thread-safe?
João Paulo Varandas
joaovarandas at inpaas.com
Mon Feb 20 15:11:44 UTC 2017
What I intended with that test, is that you can try having handlebars
evaluated in each Thread(using ThreadLocal maybe) and they will have their
own values. As long as handlebars code do not use any global objects.
By the way, that code has been executed 1.000.000 times in 3 seconds, using
range(0, 999999), so it's scaled pretty nicely in my developer machine. In
the other hand, having a new ScriptEngine for each thread may not scale so
well ...
Anyway, it was a good exercise of closures, bindings and engines. I hope
those samples helped you out achieving what you need.
Also, if it's an option, you could check out jknack's handlebars:
https://github.com/jknack/handlebars.java
Which is a Java implementation with thread-safety.
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 e: joaovarandas at inpaas.com
2017-02-20 11:54 GMT-03:00 João Paulo Varandas <joaovarandas at inpaas.com>:
> I get it now, maybe I didn't pay attention to your problem in the first
> place and was "fixated" by the thread safety idea.
>
> Nashorn has many ways to avoid those issues w/o the need for a new Engine.
> Depending on how the library works, you could use new Bindings for each
> Thread or Execution, but you may also test simply having a new Closure for
> each thread. Check it out:
>
>
> @Test
> public void testClosureThreadSafety() throws ScriptException {
> String testJsFunction = (
> " (function outerFunction(currentThreadName) {\n" +
> " print(indx.incrementAndGet() + ' ' +
> java.lang.Thread.currentThread().toString() + ': closure data is ' +
> currentThreadName) \n" +
> " function innerFunction() {\n" +
> " return currentThreadName;\n" +
> " }\n" +
> " return innerFunction;\n" +
> " })(java.lang.Thread.currentThread().toString())\n");
>
> e.getBindings(ScriptContext.GLOBAL_SCOPE).put("indx", new
> AtomicInteger(0));
> ScriptObjectMirror jsFunction = (ScriptObjectMirror)
> e.eval(testJsFunction);
>
> String mainThreadName = Thread.currentThread().toString();
>
> ThreadLocal<ScriptObjectMirror> innerjs = new ThreadLocal<ScriptObjectMirror>()
> {
> @Override
> public ScriptObjectMirror get() {
> try {
> return (ScriptObjectMirror) e.eval(testJsFunction);
> } catch(Exception e) {
> throw new RuntimeException(e);
>
> }
> }
>
> };
>
> IntConsumer invokeAndTest = i -> {
> try {
> // my main closure is unchanged
> Assert.assertEquals(mainThreadName, jsFunction.call(jsFunction));
>
> // since I have a new closure for each thread ...
> final ScriptObjectMirror innerJsFunction = innerjs.get();
> final String currentThreadName = Thread.currentThread().
> toString();
> Assert.assertEquals(currentThreadName,
> innerJsFunction.call(innerJsFunction));
>
> } catch(Exception e) {
> throw new RuntimeException(e);
> }
>
> };
> IntStream.range(0, 10).parallel().forEach(invokeAndTest);
> }
>
>
> Now we have different closures for each Thread, without the need of a new
> Engine or Bindings.
>
>
>
>
>
> João Varandas
> *Arquiteto de Soluções Cloud*
> inPaaS - Idéias em Aplicações
>
> p: +55 11 5091-2777 <+55%2011%205091-2777> m: +55 11 99889-2321
> <+55%2011%2099889-2321>
> a: Rua Nebraska, 443 - 1o Andar
> Brooklin Paulista, São Paulo, SP
> w: www.inpaas.com e: joaovarandas at inpaas.com
>
>
> 2017-02-20 11:44 GMT-03:00 Frantzius, Jörg <Joerg.Frantzius at aperto.com>:
>
>> Hi Joao,
>>
>> please see below.
>>
>> Am 20.02.2017 um 14:50 schrieb João Paulo Varandas <
>> joaovarandas at 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/jfrant
>> zius/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.
>>
>>
>>
>>
>>
>>
>>
>>
>> João Varandas
>> *Arquiteto de Soluções Cloud*
>> inPaaS - Idéias em Aplicações
>>
>> p: +55 11 5091-2777 <+55%2011%205091-2777> m: +55 11 99889-2321
>> <+55%2011%2099889-2321>
>> a: Rua Nebraska, 443 - 1o Andar
>> Brooklin Paulista, São Paulo, SP
>> w: www.inpaas.com e: joaovarandas at inpaas.com
>>
>>
>> 2017-02-20 10:32 GMT-03:00 Frantzius, Jörg <Joerg.Frantzius at 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().getEngin
>>> eByName("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().toStrin
>>> g();
>>> 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 <
>>> joaovarandas at 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?
>>>
>>>
>>>
>>>
>>>
>>> João Varandas
>>> *Arquiteto de Soluções Cloud*
>>> inPaaS - Idéias em Aplicações
>>>
>>> p: +55 11 5091-2777 <+55%2011%205091-2777> m: +55 11 99889-2321
>>> <+55%2011%2099889-2321>
>>> a: Rua Nebraska, 443 - 1o Andar
>>> Brooklin Paulista, São Paulo, SP
>>> w: www.inpaas.com e: joaovarandas at inpaas.com
>>>
>>>
>>> 2017-02-19 18:30 GMT-03:00 Frantzius, Jörg <Joerg.Frantzius at 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.Frantzius at aperto.com<mailto:Joerg.Frantzius at 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.frantzius at aperto.com<mailto:joerg.frantzius at 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
>>>>
>>>>
>>>>
>>>> ---
>>>>
>>>> Dipl. Inf. Jörg von Frantzius, Technical Director
>>>>
>>>> E-Mail joerg.frantzius at 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
>>>>
>>>>
>>>
>>> "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.frantzius at aperto.com
>>>
>>> Phone +49 30 283921-318 <+49%2030%20283921318>
>>> Fax +49 30 283921-29 <+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.frantzius at aperto.com
>>
>> Phone +49 30 283921-318 <+49%2030%20283921318>
>> Fax +49 30 283921-29 <+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"
More information about the nashorn-dev
mailing list