Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jsscripting] Synchronize context access in logger initialization #17496

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.openhab.core.automation.module.script.ScriptTransformationService.OPENHAB_TRANSFORMATION_SCRIPT;

import java.util.Arrays;
import java.util.concurrent.locks.Lock;
import java.util.stream.Collectors;

import javax.script.Compilable;
Expand All @@ -34,7 +35,7 @@
* @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 & Compilable>
class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable & Compilable & Lock>
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<T> {

private static final int STACK_TRACE_LENGTH = 5;
Expand All @@ -48,8 +49,18 @@ public DebuggingGraalScriptEngine(T delegate) {
@Override
protected void beforeInvocation() {
super.beforeInvocation();
if (logger == null) {
initializeLogger();
// OpenhabGraalJSScriptEngine::beforeInvocation will be executed after
// DebuggingGraalScriptEngine::beforeInvocation, because GraalJSScriptEngineFactory::createScriptEngine returns
// a DebuggingGraalScriptEngine instance.
// We therefore need to synchronize logger setup here and cannot rely on the synchronization in
// OpenhabGraalJSScriptEngine.
delegate.lock();
try {
if (logger == null) {
initializeLogger();
}
} finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid deadlocks
delegate.unlock();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.time.ZonedDateTime;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer;
Expand Down Expand Up @@ -69,7 +71,8 @@
* {@link Lock} for multi-thread synchronization; globals and openhab-js injection code caching
*/
public class OpenhabGraalJSScriptEngine
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine> {
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine>
implements Lock {

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

return new InputStreamReader(ioStream);
}

@Override
public void lock() {
lock.lock();
}

@Override
public void lockInterruptibly() throws InterruptedException {
lock.lockInterruptibly();
}

@Override
public boolean tryLock() {
return lock.tryLock();
}

@Override
public boolean tryLock(long l, TimeUnit timeUnit) throws InterruptedException {
return lock.tryLock(l, timeUnit);
}

@Override
public void unlock() {
lock.unlock();
}

@Override
public Condition newCondition() {
return lock.newCondition();
}
}