From 2c4dccf3b9aa056a04d722681658e7fe27cf5488 Mon Sep 17 00:00:00 2001 From: Bart Maertens Date: Sat, 23 Mar 2024 11:42:17 +0100 Subject: [PATCH] fix ConcurrentModificationExceptions in script transforms. #3729 --- .../0071-concurrent-scripts-3729.hpl | 292 ++++++++++++++++++ .../main-0071-concurrent-scripts-3729.hwf | 92 ++++++ .../pipeline/transforms/script/Script.java | 2 +- .../transforms/script/ScriptDialog.java | 4 +- .../transforms/script/ScriptUtils.java | 38 ++- 5 files changed, 415 insertions(+), 13 deletions(-) create mode 100644 integration-tests/transforms/0071-concurrent-scripts-3729.hpl create mode 100644 integration-tests/transforms/main-0071-concurrent-scripts-3729.hwf diff --git a/integration-tests/transforms/0071-concurrent-scripts-3729.hpl b/integration-tests/transforms/0071-concurrent-scripts-3729.hpl new file mode 100644 index 00000000000..7e94935b8cd --- /dev/null +++ b/integration-tests/transforms/0071-concurrent-scripts-3729.hpl @@ -0,0 +1,292 @@ + + + + + 0071-concurrent-scripts-3729 + Y + + + + Normal + + + N + 1000 + 100 + - + 2024/03/23 11:24:08.233 + - + 2024/03/23 11:24:08.233 + + + + + + ECMAScript + preview + Y + + + Groovy + preview + Y + + + Python + preview + Y + + + generate 10k rows + ECMAScript + Y + + + generate 10k rows + Groovy + Y + + + generate 10k rows + Python + Y + + + + ECMAScript + SuperScript + + Y + + 10 + + none + + + + + -1 + id + -1 + id + N + N + Integer + + + -1 + name + -1 + name + N + N + String + + + ECMAScript + + + + + + 384 + 96 + + + + Groovy + SuperScript + + Y + + 10 + + none + + + + + -1 + id + -1 + id + N + N + Integer + + + -1 + name + -1 + name + N + N + String + + + Groovy + + + + + + 384 + 208 + + + + Python + SuperScript + + Y + + 10 + + none + + + + + -1 + id + -1 + id + N + N + Integer + + + -1 + name + -1 + name + N + N + String + + + python + + + + + + 384 + 320 + + + + generate 10k rows + RowGenerator + + N + + 1 + + none + + + + + 5000 + FiveSecondsAgo + 10000 + N + now + + + 192 + 208 + + + + preview + Dummy + + Y + + 1 + + none + + + + + 576 + 208 + + + + + + diff --git a/integration-tests/transforms/main-0071-concurrent-scripts-3729.hwf b/integration-tests/transforms/main-0071-concurrent-scripts-3729.hwf new file mode 100644 index 00000000000..1ef06dec23f --- /dev/null +++ b/integration-tests/transforms/main-0071-concurrent-scripts-3729.hwf @@ -0,0 +1,92 @@ + + + + main-0071-concurrent-scripts-3729 + Y + + + + - + 2024/03/23 11:24:45.685 + - + 2024/03/23 11:24:45.685 + + + + + Start + + SPECIAL + + 1 + 12 + 60 + 0 + 0 + N + 0 + 1 + N + 50 + 50 + + + + 0071-concurrent-scripts-3729.hpl + + PIPELINE + + N + N + N + N + N + N + ${PROJECT_HOME}/0071-concurrent-scripts-3729.hpl + + + Basic + + Y + + N + local + N + N + Y + N + 224 + 48 + + + + + + Start + 0071-concurrent-scripts-3729.hpl + Y + Y + Y + + + + + + diff --git a/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/Script.java b/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/Script.java index 01deb31c766..95413b1348c 100644 --- a/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/Script.java +++ b/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/Script.java @@ -688,7 +688,7 @@ public boolean init() { } } try { - data.engine = ScriptUtils.createNewScriptEngineByLanguage(meta.getLanguageName()); + data.engine = ScriptUtils.getInstance().getScriptEngineByName(meta.getLanguageName()); } catch (Exception e) { log.logError("Error obtaining scripting engine for language " + meta.getLanguageName(), e); } diff --git a/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptDialog.java b/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptDialog.java index de0a6f09874..1abb5c3c27a 100644 --- a/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptDialog.java +++ b/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptDialog.java @@ -236,7 +236,7 @@ public String open() { fdlEngines.top = new FormAttachment(wTransformName, margin); wlEngines.setLayoutData(fdlEngines); wEngines = new CCombo(shell, SWT.LEFT | SWT.READ_ONLY | SWT.BORDER); - List scriptEngineNames = ScriptUtils.getScriptLanguageNames(); + List scriptEngineNames = ScriptUtils.getInstance().getScriptLanguageNames(); if (scriptEngineNames != null) { Collections.sort(scriptEngineNames); @@ -775,7 +775,7 @@ public void setPosition() { public void getData() { String engineName = input.getLanguageName(); if (StringUtils.isEmpty(engineName) - || !ScriptUtils.getScriptLanguageNames().contains(engineName)) { + || !ScriptUtils.getInstance().getScriptLanguageNames().contains(engineName)) { wEngines.select(0); } else { wEngines.setText(engineName); diff --git a/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptUtils.java b/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptUtils.java index a913ab75727..3bc7ec71a93 100644 --- a/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptUtils.java +++ b/plugins/transforms/script/src/main/java/org/apache/hop/pipeline/transforms/script/ScriptUtils.java @@ -19,9 +19,9 @@ package org.apache.hop.pipeline.transforms.script; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentSkipListMap; import javax.script.ScriptEngine; import javax.script.ScriptEngineFactory; import javax.script.ScriptEngineManager; @@ -31,9 +31,27 @@ public class ScriptUtils { private static ScriptEngineManager scriptEngineManager; - private static final Map> languageFactoryMap = new HashMap<>(); + private static ScriptUtils instance; + private static final Map> languageFactoryMap = new ConcurrentSkipListMap<>(); private static List engineFactories = null; + private ScriptUtils(){ + scriptEngineManager = getScriptEngineManager(); + List scriptLanguageNames = getScriptLanguageNames(); + for(String scriptLanguageName : scriptLanguageNames){ + createNewScriptEngine(scriptLanguageName); + } + + populateEngineFactoryMap(); + } + + public static ScriptUtils getInstance(){ + if(instance == null){ + instance = new ScriptUtils(); + } + return instance; + } + /** * Instantiates the right scripting language interpreter, falling back to Groovy for backward * compatibility @@ -41,23 +59,20 @@ public class ScriptUtils { * @param engineName * @return the desired ScriptEngine, or null if none can be found */ - public static ScriptEngine createNewScriptEngine(String engineName) { + private static ScriptEngine createNewScriptEngine(String engineName) { ScriptEngine scriptEngine = getScriptEngineManager().getEngineByName(engineName); if (scriptEngine == null) { // falls back to Groovy - scriptEngine = getScriptEngineManager().getEngineByName("groovy"); + scriptEngine = scriptEngineManager.getEngineByName("groovy"); } return scriptEngine; } - public static ScriptEngine createNewScriptEngineByLanguage(String languageName) + private static ScriptEngine createNewScriptEngineByLanguage(String languageName) throws HopException { ScriptEngine scriptEngine = null; - if (engineFactories == null) { - populateEngineFactoryMap(); - } List factories = languageFactoryMap.get(languageName); if (factories != null) { for (ScriptEngineFactory factory : factories) { @@ -84,14 +99,13 @@ public static ScriptEngine createNewScriptEngineByLanguage(String languageName) return scriptEngine; } - public static ScriptEngineManager getScriptEngineManager() { + private static ScriptEngineManager getScriptEngineManager() { if (scriptEngineManager == null) { System.setProperty( "org.jruby.embed.localvariable.behavior", "persistent"); // required for JRuby, transparent // for others scriptEngineManager = new ScriptEngineManager(ScriptUtils.class.getClassLoader()); - populateEngineFactoryMap(); } return scriptEngineManager; } @@ -119,4 +133,8 @@ private static void populateEngineFactoryMap() { } } } + + public ScriptEngine getScriptEngineByName(String scriptLanguegeName){ + return scriptEngineManager.getEngineByName(scriptLanguegeName); + } }