demus/openhab-addons
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-addons synced 2026-07-29 12:34:21 +02:00
Code Issues Packages Projects Releases Wiki Activity

[jsscripting] Fix memory leak caused by GraalJSScriptEngine not closed properly (#18226)

Browse Source 
* [jsscripting] Close GraalJSScriptEngine when closing OpenhabGraalJSScriptEngine

Signed-off-by: Florian Hotze <dev@florianhotze.com>
This commit is contained in:
Florian Hotze
2025-08-09 13:18:18 +02:00
committed by GitHub
parent f9f2a24191
commit 7433801d22
4 changed files with 93 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java
+15 -8
View File
@@ -73,14 +73,22 @@ class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoClosea
// 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: {}", stringifyThrowable(cause));
} else if (cause instanceof PolyglotException) {
logger.error("Failed to execute script: {}", stringifyThrowable(cause));
if (cause instanceof IllegalArgumentException || cause instanceof PolyglotException) {
String strT = stringifyThrowable(cause);
logger.error("Failed to execute script: {}", strT);
}
return e;
}
@Override
public void close() {
try {
super.close();
} catch (Exception e) {
logger.warn("Ignorable exception during close: {}", stringifyThrowable(e));
}
}
private String stringifyThrowable(Throwable throwable) {
String message = throwable.getMessage();
StackTraceElement[] stackTraceElements = throwable.getStackTrace();
@@ -106,10 +114,9 @@ class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoClosea
identifier = fileName.toString().replaceAll("^.*[/\\\\]", "");
} else if (ruleUID != null) {
identifier = ruleUID.toString();
} else if (ohEngineIdentifier != null) {
if (ohEngineIdentifier.toString().startsWith(OPENHAB_TRANSFORMATION_SCRIPT)) {
identifier = ohEngineIdentifier.toString().replaceAll(OPENHAB_TRANSFORMATION_SCRIPT, "transformation.");
}
} else if (ohEngineIdentifier != null
&& ohEngineIdentifier.toString().startsWith(OPENHAB_TRANSFORMATION_SCRIPT)) {
identifier = ohEngineIdentifier.toString().replaceAll(OPENHAB_TRANSFORMATION_SCRIPT, "transformation.");
}
logger = LoggerFactory.getLogger("org.openhab.automation.script.javascript." + identifier);
bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java
+24 -3
View File
@@ -54,6 +54,7 @@ import org.openhab.automation.jsscripting.internal.fs.PrefixedSeekableByteChanne
import org.openhab.automation.jsscripting.internal.fs.ReadOnlySeekableByteArrayChannel;
import org.openhab.automation.jsscripting.internal.fs.watch.JSDependencyTracker;
import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable;
import org.openhab.automation.jsscripting.internal.scriptengine.helper.LifecycleTracker;
import org.openhab.core.automation.module.script.ScriptExtensionAccessor;
import org.openhab.core.items.Item;
import org.openhab.core.library.types.QuantityType;
@@ -138,6 +139,7 @@ public class OpenhabGraalJSScriptEngine
/** {@link Lock} synchronization of multi-thread access */
private final Lock lock = new ReentrantLock();
private final JSRuntimeFeatures jsRuntimeFeatures;
private final LifecycleTracker lifecycleTracker = new LifecycleTracker();
private final GraalJSScriptEngineConfiguration configuration;
// these fields start as null because they are populated on first use
@@ -145,6 +147,7 @@ public class OpenhabGraalJSScriptEngine
private String engineIdentifier = "<uninitialized>";
private boolean initialized = false;
private boolean closed = false;
/**
* Creates an implementation of ScriptEngine {@code (& Invocable)}, wrapping the contained engine,
@@ -283,7 +286,7 @@ public class OpenhabGraalJSScriptEngine
scriptDependencyListener = localScriptDependencyListener;
ScriptExtensionModuleProvider scriptExtensionModuleProvider = new ScriptExtensionModuleProvider(
scriptExtensionAccessor, lock);
scriptExtensionAccessor, lock, lifecycleTracker);
// Wrap the "require" function to also allow loading modules from the ScriptExtensionModuleProvider
Function<Function<Object[], Object>, Function<String, Object>> wrapRequireFn = originalRequireFn -> moduleName -> scriptExtensionModuleProvider
@@ -345,8 +348,26 @@ public class OpenhabGraalJSScriptEngine
}
@Override
public void close() {
jsRuntimeFeatures.close();
public void close() throws Exception {
if (closed) {
logger.debug("Engine '{}' is already disposed and closed.", engineIdentifier);
return;
}
lock.lock();
try {
try {
jsRuntimeFeatures.close();
this.lifecycleTracker.dispose();
} finally {
logger.debug("Engine '{}' disposed.", engineIdentifier);
super.close();
logger.debug("Engine '{}' closed.", engineIdentifier);
}
} finally {
closed = true;
lock.unlock();
}
}
/**
bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/ScriptExtensionModuleProvider.java
+12 -7
View File
@@ -22,6 +22,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Source;
import org.graalvm.polyglot.Value;
import org.openhab.automation.jsscripting.internal.scriptengine.helper.LifecycleTracker;
import org.openhab.automation.jsscripting.internal.threading.ThreadsafeWrappingScriptedAutomationManagerDelegate;
import org.openhab.core.automation.module.script.ScriptExtensionAccessor;
import org.openhab.core.automation.module.script.rulesupport.shared.ScriptedAutomationManager;
@@ -30,22 +31,25 @@ import org.openhab.core.automation.module.script.rulesupport.shared.ScriptedAuto
* Class providing script extensions via CommonJS modules (with module name `@runtime`).
*
* @author Jonathan Gilbert - Initial contribution
* @author Florian Hotze - Pass in lock object for multi-thread synchronization; Switch to {@link Lock} for multi-thread
* synchronization
* @author Florian Hotze - Pass in a lock object for multi-thread synchronisation
* @author Florian Hotze - Switch to {@link Lock} for multi-thread synchronisation
* @author Florian Hotze - Overwrite lifecycleTracker with our own implementation
*/
@NonNullByDefault
public class ScriptExtensionModuleProvider {
private static final String RUNTIME_MODULE_PREFIX = "@runtime";
private static final String DEFAULT_MODULE_NAME = "Defaults";
private final Lock lock;
private final LifecycleTracker lifecycleTracker;
private final ScriptExtensionAccessor scriptExtensionAccessor;
public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor, Lock lock) {
public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor, Lock lock,
LifecycleTracker lifecycleTracker) {
this.scriptExtensionAccessor = scriptExtensionAccessor;
this.lock = lock;
this.lifecycleTracker = lifecycleTracker;
}
public ModuleLocator locatorFor(Context ctx, String engineIdentifier) {
@@ -68,6 +72,7 @@ public class ScriptExtensionModuleProvider {
if (DEFAULT_MODULE_NAME.equals(name)) {
symbols = scriptExtensionAccessor.findDefaultPresets(scriptIdentifier);
symbols.put("lifecycleTracker", lifecycleTracker);
} else {
symbols = scriptExtensionAccessor.findPreset(name, scriptIdentifier);
}
@@ -102,9 +107,9 @@ public class ScriptExtensionModuleProvider {
Map<String, Object> rv = new HashMap<>(values);
for (Map.Entry<String, Object> entry : rv.entrySet()) {
if (entry.getValue() instanceof ScriptedAutomationManager) {
entry.setValue(new ThreadsafeWrappingScriptedAutomationManagerDelegate(
(ScriptedAutomationManager) entry.getValue(), lock));
if (entry.getValue() instanceof ScriptedAutomationManager scriptedAutomationManager) {
entry.setValue(
new ThreadsafeWrappingScriptedAutomationManagerDelegate(scriptedAutomationManager, lock));
}
}
bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/helper/LifecycleTracker.java
+42
View File
@@ -0,0 +1,42 @@
/*
* Copyright (c) 2010-2025 Contributors to the openHAB project
*
* See the NOTICE file(s) distributed with this work for additional
* information.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.openhab.automation.jsscripting.internal.scriptengine.helper;
import java.util.ArrayList;
import java.util.List;
import org.eclipse.jdt.annotation.NonNullByDefault;
/**
* LifecycleTracker implementation
*
* <p>
* We can't use core's lifecycle tracker for JS Scripting, because its dispose hooks are called after the engine has
* been closed (which will not work).
*
* @author Florian Hotze - Initial contribution
*/
@NonNullByDefault
public class LifecycleTracker {
private List<Runnable> disposables = new ArrayList<>();
public void addDisposeHook(Runnable disposable) {
disposables.add(disposable);
}
public void dispose() {
for (Runnable disposable : disposables) {
disposable.run();
}
}
}