[jsscripting] Fix memory leak on script execution failure (#16578)

Make engineIdentifier a instance field to ease debugging.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
This commit is contained in:
Florian Hotze 2024-03-27 21:43:07 +01:00 committed by Ciprian Pascu
parent 855ce4cfb1
commit 30d32f9357
3 changed files with 24 additions and 5 deletions

View File

@ -12,6 +12,9 @@
*/
package org.openhab.automation.jsscripting.internal;
import java.util.Arrays;
import java.util.stream.Collectors;
import javax.script.Invocable;
import javax.script.ScriptContext;
import javax.script.ScriptEngine;
@ -26,11 +29,13 @@ import org.slf4j.LoggerFactory;
* Wraps ScriptEngines provided by Graal to provide error messages and stack traces for scripts.
*
* @author Jonathan Gilbert - Initial contribution
* @author Florian Hotze - Improve logger name, Fix memory leak caused by exception logging
*/
class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable>
extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable<T> {
private static final String SCRIPT_TRANSFORMATION_ENGINE_IDENTIFIER = "openhab-transformation-script-";
private static final int STACK_TRACE_LENGTH = 5;
private @Nullable Logger logger;
@ -49,15 +54,26 @@ class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoClosea
@Override
public Exception afterThrowsInvocation(Exception e) {
Throwable cause = e.getCause();
// OPS4J Pax Logging holds a reference to the exception, which causes the OpenhabGraalJSScriptEngine to not be
// removed from heap by garbage collection and causing a memory leak.
// Therefore, don't pass the exceptions itself to the logger, but only their message!
if (cause instanceof IllegalArgumentException) {
logger.error("Failed to execute script:", e);
}
if (cause instanceof PolyglotException) {
logger.error("Failed to execute script:", cause);
logger.error("Failed to execute script: {}", stringifyThrowable(cause));
} else if (cause instanceof PolyglotException) {
logger.error("Failed to execute script: {}", stringifyThrowable(cause));
}
return e;
}
private String stringifyThrowable(Throwable throwable) {
String message = throwable.getMessage();
StackTraceElement[] stackTraceElements = throwable.getStackTrace();
String stackTrace = Arrays.stream(stackTraceElements).limit(STACK_TRACE_LENGTH)
.map(t -> " at " + t.toString()).collect(Collectors.joining(System.lineSeparator()))
+ System.lineSeparator() + " ... " + stackTraceElements.length + " more";
return (message != null) ? message + System.lineSeparator() + stackTrace : stackTrace;
}
/**
* Initializes the logger.
* This cannot be done on script engine creation because the context variables are not yet initialized.

View File

@ -135,6 +135,7 @@ public class OpenhabGraalJSScriptEngine
// these fields start as null because they are populated on first use
private @Nullable Consumer<String> scriptDependencyListener;
private String engineIdentifier; // this field is very helpful for debugging, please do not remove it
private boolean initialized = false;
private final boolean injectionEnabled;
@ -242,6 +243,7 @@ public class OpenhabGraalJSScriptEngine
if (localEngineIdentifier == null) {
throw new IllegalStateException("Failed to retrieve engine identifier from engine bindings");
}
this.engineIdentifier = localEngineIdentifier;
ScriptExtensionAccessor scriptExtensionAccessor = (ScriptExtensionAccessor) ctx
.getAttribute(CONTEXT_KEY_EXTENSION_ACCESSOR);
@ -250,7 +252,7 @@ public class OpenhabGraalJSScriptEngine
}
Consumer<String> localScriptDependencyListener = (Consumer<String>) ctx
.getAttribute("oh.dependency-listener"/* CONTEXT_KEY_DEPENDENCY_LISTENER */);
.getAttribute(CONTEXT_KEY_DEPENDENCY_LISTENER);
if (localScriptDependencyListener == null) {
LOGGER.warn(
"Failed to retrieve script script dependency listener from engine bindings. Script dependency tracking will be disabled.");

View File

@ -1,2 +1,3 @@
# Please check here how to add suppressions https://maven.apache.org/plugins/maven-pmd-plugin/examples/violation-exclusions.html
org.openhab.automation.jsscripting.internal.OpenhabGraalJSScriptEngine=UnusedPrivateField
org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable=AvoidThrowingNullPointerException,AvoidCatchingNPE