Found in the review of #132; the bug predates that PR.
What happens
ScriptedConfiguration.getGroovyScriptEngine() stores the new engine in the groovyScriptEngine field (ScriptedConfiguration.java:797) before initializeCustomizer() runs (:800). It has to: getCustomizerClass() (:727) calls getGroovyScriptEngine() again on the same thread. That has two consequences:
- The customizer fails.
initializeCustomizer() (:811) wraps the exception in a ConnectorException and rethrows it, but nothing clears the field. Every later caller takes the fast path at :786 and runs scripts on an engine whose customizer never ran. In the scripted REST connector, ScriptedRESTConfiguration.getHttpClient then fails with an NPE on initClosure (ScriptedRESTConfiguration.groovy:218), which the customizer was supposed to set, and the original customizer error is lost.
- Another thread arrives during customization. A thread that reads the field at
:786 while the first thread is still inside the customizer gets the engine without its customizations.
Making the field volatile (#132) does not change either case, because the field is still published before the customization is done.
Expected
Other threads only ever see a customized engine. If the customizer fails, the next call retries the customization instead of silently using an uncustomized engine.
Found in the review of #132; the bug predates that PR.
What happens
ScriptedConfiguration.getGroovyScriptEngine()stores the new engine in thegroovyScriptEnginefield (ScriptedConfiguration.java:797) beforeinitializeCustomizer()runs (:800). It has to:getCustomizerClass()(:727) callsgetGroovyScriptEngine()again on the same thread. That has two consequences:initializeCustomizer()(:811) wraps the exception in aConnectorExceptionand rethrows it, but nothing clears the field. Every later caller takes the fast path at:786and runs scripts on an engine whose customizer never ran. In the scripted REST connector,ScriptedRESTConfiguration.getHttpClientthen fails with an NPE oninitClosure(ScriptedRESTConfiguration.groovy:218), which the customizer was supposed to set, and the original customizer error is lost.:786while the first thread is still inside the customizer gets the engine without its customizations.Making the field
volatile(#132) does not change either case, because the field is still published before the customization is done.Expected
Other threads only ever see a customized engine. If the customizer fails, the next call retries the customization instead of silently using an uncustomized engine.