Skip to content

Commit

Permalink
[jsscripting] Fix multi-thread access requested by logger initializat…
Browse files Browse the repository at this point in the history
…ion (openhab#16497)

* [jsscripting] Fix multi-threading issue with logger initialization

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
  • Loading branch information
florian-h05 authored and austvik committed Mar 27, 2024
1 parent b798ee0 commit e7099f6
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package org.openhab.automation.jsscripting.internal;

import javax.script.Invocable;
import javax.script.ScriptContext;
import javax.script.ScriptEngine;

import org.eclipse.jdt.annotation.Nullable;
Expand All @@ -38,11 +39,15 @@ public DebuggingGraalScriptEngine(T delegate) {
}

@Override
public Exception afterThrowsInvocation(Exception e) {
protected void beforeInvocation() {
super.beforeInvocation();
if (logger == null) {
initializeLogger();
}
}

@Override
public Exception afterThrowsInvocation(Exception e) {
Throwable cause = e.getCause();
if (cause instanceof IllegalArgumentException) {
logger.error("Failed to execute script:", e);
Expand All @@ -59,9 +64,10 @@ public Exception afterThrowsInvocation(Exception e) {
* Therefore, the logger needs to be initialized on the first use after script engine creation.
*/
private void initializeLogger() {
Object fileName = delegate.getContext().getAttribute("javax.script.filename");
Object ruleUID = delegate.getContext().getAttribute("ruleUID");
Object ohEngineIdentifier = delegate.getContext().getAttribute("oh.engine-identifier");
ScriptContext ctx = delegate.getContext();
Object fileName = ctx.getAttribute("javax.script.filename");
Object ruleUID = ctx.getAttribute("ruleUID");
Object ohEngineIdentifier = ctx.getAttribute("oh.engine-identifier");

String identifier = "stack";
if (fileName != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ public class OpenhabGraalJSScriptEngine
private final JSRuntimeFeatures jsRuntimeFeatures;

// these fields start as null because they are populated on first use
private @Nullable String engineIdentifier;
private @Nullable Consumer<String> scriptDependencyListener;

private boolean initialized = false;
Expand Down Expand Up @@ -243,20 +242,20 @@ protected void beforeInvocation() {
if (localEngineIdentifier == null) {
throw new IllegalStateException("Failed to retrieve engine identifier from engine bindings");
}
engineIdentifier = localEngineIdentifier;

ScriptExtensionAccessor scriptExtensionAccessor = (ScriptExtensionAccessor) ctx
.getAttribute(CONTEXT_KEY_EXTENSION_ACCESSOR);
if (scriptExtensionAccessor == null) {
throw new IllegalStateException("Failed to retrieve script extension accessor from engine bindings");
}

scriptDependencyListener = (Consumer<String>) ctx
Consumer<String> localScriptDependencyListener = (Consumer<String>) ctx
.getAttribute("oh.dependency-listener"/* CONTEXT_KEY_DEPENDENCY_LISTENER */);
if (scriptDependencyListener == null) {
if (localScriptDependencyListener == null) {
LOGGER.warn(
"Failed to retrieve script script dependency listener from engine bindings. Script dependency tracking will be disabled.");
}
scriptDependencyListener = localScriptDependencyListener;

ScriptExtensionModuleProvider scriptExtensionModuleProvider = new ScriptExtensionModuleProvider(
scriptExtensionAccessor, lock);
Expand Down Expand Up @@ -317,7 +316,7 @@ public void close() {
* @param path a root path
* @return whether the given path is a node root directory
*/
private boolean isRootNodePath(Path path) {
private static boolean isRootNodePath(Path path) {
return path.startsWith(path.getRoot().resolve(NODE_DIR));
}

Expand All @@ -328,7 +327,7 @@ private boolean isRootNodePath(Path path) {
* @param path a root path, e.g. C:\node_modules\foo.js
* @return the class resource path for loading local modules
*/
private String nodeFileToResource(Path path) {
private static String nodeFileToResource(Path path) {
return "/" + path.subpath(0, path.getNameCount()).toString().replace('\\', '/');
}

Expand Down

0 comments on commit e7099f6

Please sign in to comment.