Closures not thread-safe?
João Paulo Varandas
joaovarandas at inpaas.com
Mon Feb 20 14:54:28 UTC 2017
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 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: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/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.
>
>
>
>
>
>
>
>
> 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().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 <
>> 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