Skip to content

Commit 63f1646

Browse files
committed
[jsscripting] Synchronize context access in logger initialisation to avoid illegal multi-thread access
Fixes openhab#17494. Signed-off-by: Florian Hotze <[email protected]>
1 parent c023801 commit 63f1646

File tree

2 files changed

+49
-4
lines changed

2 files changed

+49
-4
lines changed

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java

+15-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import static org.openhab.core.automation.module.script.ScriptTransformationService.OPENHAB_TRANSFORMATION_SCRIPT;
1616

1717
import java.util.Arrays;
18+
import java.util.concurrent.locks.Lock;
1819
import java.util.stream.Collectors;
1920

2021
import javax.script.Compilable;
@@ -34,7 +35,7 @@
3435
* @author Jonathan Gilbert - Initial contribution
3536
* @author Florian Hotze - Improve logger name, Fix memory leak caused by exception logging
3637
*/
37-
class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable & Compilable>
38+
class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable & Compilable & Lock>
3839
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<T> {
3940

4041
private static final int STACK_TRACE_LENGTH = 5;
@@ -48,8 +49,19 @@ public DebuggingGraalScriptEngine(T delegate) {
4849
@Override
4950
protected void beforeInvocation() {
5051
super.beforeInvocation();
51-
if (logger == null) {
52-
initializeLogger();
52+
// OpenhabGraalJSScriptEngine::beforeInvocation will be executed after
53+
// DebuggingGraalScriptEngine::beforeInvocation, because GraalJSScriptEngineFactory::createScriptEngine returns
54+
// a DebuggingGraalScriptEngine instance.
55+
// We therefore need to synchronize locker setup here and cannot rely on the synchronization in
56+
// OpenhabGraalJSScriptEngine.
57+
delegate.lock();
58+
try {
59+
if (logger == null) {
60+
initializeLogger();
61+
}
62+
} finally { // Make sure that Lock is unlocked regardless of an exception is thrown or not to avoid
63+
// deadlocks
64+
delegate.unlock();
5365
}
5466
}
5567

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java

+34-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import java.time.ZonedDateTime;
3333
import java.util.Map;
3434
import java.util.Set;
35+
import java.util.concurrent.TimeUnit;
36+
import java.util.concurrent.locks.Condition;
3537
import java.util.concurrent.locks.Lock;
3638
import java.util.concurrent.locks.ReentrantLock;
3739
import java.util.function.Consumer;
@@ -69,7 +71,8 @@
6971
* {@link Lock} for multi-thread synchronization; globals and openhab-js injection code caching
7072
*/
7173
public class OpenhabGraalJSScriptEngine
72-
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine> {
74+
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine>
75+
implements Lock {
7376

7477
private static final Logger LOGGER = LoggerFactory.getLogger(OpenhabGraalJSScriptEngine.class);
7578
private static final Source GLOBAL_SOURCE;
@@ -346,4 +349,34 @@ private static Reader getFileAsReader(String fileName) throws IOException {
346349

347350
return new InputStreamReader(ioStream);
348351
}
352+
353+
@Override
354+
public void lock() {
355+
lock.lock();
356+
}
357+
358+
@Override
359+
public void lockInterruptibly() throws InterruptedException {
360+
lock.lockInterruptibly();
361+
}
362+
363+
@Override
364+
public boolean tryLock() {
365+
return lock.tryLock();
366+
}
367+
368+
@Override
369+
public boolean tryLock(long l, TimeUnit timeUnit) throws InterruptedException {
370+
return lock.tryLock(l, timeUnit);
371+
}
372+
373+
@Override
374+
public void unlock() {
375+
lock.unlock();
376+
}
377+
378+
@Override
379+
public Condition newCondition() {
380+
return lock.newCondition();
381+
}
349382
}

0 commit comments

Comments
 (0)