Skip to content

Commit

Permalink
[GR-52530] Fixed disabled truffle logger combined with contexts with …
Browse files Browse the repository at this point in the history
…default log levels.

PullRequest: graal/17367
  • Loading branch information
tzezula committed Mar 27, 2024
2 parents e53dac5 + 7dead49 commit 5d65e5a
Show file tree
Hide file tree
Showing 6 changed files with 359 additions and 284 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -462,11 +462,12 @@ public Charset detectEncoding(TruffleFile file, String mimeType) {

@Override
public void configureLoggers(Object vmObject, Map<String, Level> logLevels, Object... loggers) {
for (Object loggerCache : loggers) {
for (Object logger : loggers) {
TruffleLogger.LoggerCache loggerCache = (TruffleLogger.LoggerCache) logger;
if (logLevels == null) {
((TruffleLogger.LoggerCache) loggerCache).removeLogLevelsForVMObject(vmObject);
} else {
((TruffleLogger.LoggerCache) loggerCache).addLogLevelsForVMObject(vmObject, logLevels);
loggerCache.removeLogLevelsForVMObject(vmObject);
} else if (!logLevels.isEmpty() || !ENGINE.isContextBoundLogger(loggerCache.getSPI())) {
loggerCache.addLogLevelsForVMObject(vmObject, logLevels);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;
import java.util.logging.Handler;
Expand Down Expand Up @@ -819,8 +821,8 @@ private void updateLevelNum(boolean singleContext) {
} else {
value = DEFAULT_VALUE;
}
setLevelNum(value);
if (children != null) {
boolean changed = setLevelNum(value);
if (changed && children != null) {
for (ChildLoggerRef ref : children) {
final TruffleLogger logger = ref.get();
if (logger != null) {
Expand Down Expand Up @@ -1152,9 +1154,8 @@ private Set<String> collectRemovedLevels() {

private void reconfigure(final Map<String, Level> addedLevels, final Set<String> toRemove) {
assert Thread.holdsLock(this);
assert !addedLevels.isEmpty() || !toRemove.isEmpty();
final Collection<String> loggersWithRemovedLevels = new HashSet<>();
final Collection<String> loggersWithChangedLevels = new HashSet<>();
final Collection<String> loggersWithRemovedLevels = new TreeSet<>(Comparator.reverseOrder());
final Collection<String> loggersWithChangedLevels = new TreeSet<>();
boolean singleContext = activeContexts.size() <= 1;
effectiveLevels = computeEffectiveLevels(
effectiveLevels,
Expand All @@ -1164,9 +1165,6 @@ private void reconfigure(final Map<String, Level> addedLevels, final Set<String>
singleContext,
loggersWithRemovedLevels,
loggersWithChangedLevels);
if (singleContext && !loggersWithRemovedLevels.isEmpty()) {
loggersWithChangedLevels.addAll(effectiveLevels.keySet());
}
for (String loggerName : loggersWithRemovedLevels) {
final TruffleLogger logger = getLogger(loggerName);
if (logger != null) {
Expand Down Expand Up @@ -1257,18 +1255,50 @@ private static Map<String, Level> computeEffectiveLevels(
}
}
}
Map<String, Level> addedWithDefaults = new HashMap<>(added);
if (!singleContext) {
// In a multi context scenario there can be a logger with higher effective log level
// than a default one. When the newly configured context does not specify log level
// explicitly the effective log level of such a logger needs to be set to the
// default
// level.
Map<String, Level> toAdd;
if (singleContext) {
if (!newEffectiveLevels.isEmpty()) {
// During the transition from a multi-context to a single context, log levels
// may need to be adjusted. In a multi-context scenario, log levels can be
// escalated to the default log level if a logger has a higher log level than
// the default level, such as Level.OFF. Additionally, log levels may be
// inherited from a parent logger if the parent has a lower log level than the
// logger's own level.
ContextWeakReference ref = contexts.iterator().next();
Map<String, Level> config = ref.configuredLoggers;
for (Map.Entry<String, Level> e : newEffectiveLevels.entrySet()) {
String loggerName = e.getKey();
Level level = config.get(loggerName);
assert level != null;
e.setValue(level);
changedLevels.add(loggerName);
}
}
toAdd = added;
} else {
// In a multi-context scenario, if a newly registered context specifies a logger
// with a higher effective log level than the default level, for example Level.OFF,
// and this logger is not configured by already existing contexts, we need to use
// the default level to ensure that loggers for existing contexts are not disabled.
toAdd = new HashMap<>();
for (Map.Entry<String, Level> e : added.entrySet()) {
String loggerName = e.getKey();
Level loggerLevel = e.getValue();
if (!newEffectiveLevels.containsKey(loggerName)) {
loggerLevel = min(loggerLevel, Level.INFO);
}
toAdd.put(loggerName, loggerLevel);
}
// In a multi-context scenario, it's possible to encounter a logger with a higher
// effective log level than the default level inherited from an existing context,
// for example Level.OFF. In cases where the newly configured context does not
// explicitly specify a log level for this logger, it's necessary to ensure that the
// effective log level of the logger is set to the default level.
for (String loggerName : newEffectiveLevels.keySet()) {
addedWithDefaults.putIfAbsent(loggerName, Level.INFO);
toAdd.putIfAbsent(loggerName, Level.INFO);
}
}
for (Map.Entry<String, Level> addedLevel : addedWithDefaults.entrySet()) {
for (Map.Entry<String, Level> addedLevel : toAdd.entrySet()) {
final String loggerName = addedLevel.getKey();
final Level loggerLevel = addedLevel.getValue();
final Level currentLevel = newEffectiveLevels.get(loggerName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,9 +555,7 @@ private PolyglotContextImpl() {
* Instruments can add loggers, and so configuration of loggers for this context must be
* done after instruments are created.
*/
if (!config.logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(this, config.logLevels, getAllLoggers());
}
EngineAccessor.LANGUAGE.configureLoggers(this, config.logLevels, getAllLoggers());
}

/*
Expand Down Expand Up @@ -588,9 +586,7 @@ private PolyglotContextImpl() {
this.exitMessage = this.parent.exitMessage;
this.contextBoundLoggers = this.parent.contextBoundLoggers;
this.threadLocalActions = new PolyglotThreadLocalActions(this);
if (!parent.config.logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(this, parent.config.logLevels, getAllLoggers());
}
EngineAccessor.LANGUAGE.configureLoggers(this, parent.config.logLevels, getAllLoggers());
this.contexts = createContextArray();
this.subProcesses = new HashSet<>();
// notifyContextCreated() is called after spiContext.impl is set to this.
Expand Down Expand Up @@ -2842,12 +2838,10 @@ private boolean finishClose(boolean cancelOrExitOperation, boolean notifyInstrum
}
}
if (parent == null) {
if (!this.config.logLevels.isEmpty()) {
Object defaultLoggers = EngineAccessor.LANGUAGE.getDefaultLoggers();
Object engineLoggers = engine.getEngineLoggers();
Object[] loggersToRecompute = engineLoggers != null ? new Object[]{defaultLoggers, engineLoggers} : new Object[]{defaultLoggers};
EngineAccessor.LANGUAGE.configureLoggers(this, null, loggersToRecompute);
}
Object defaultLoggers = EngineAccessor.LANGUAGE.getDefaultLoggers();
Object engineLoggers = engine.getEngineLoggers();
Object[] loggersToRecompute = engineLoggers != null ? new Object[]{defaultLoggers, engineLoggers} : new Object[]{defaultLoggers};
EngineAccessor.LANGUAGE.configureLoggers(this, null, loggersToRecompute);
if (this.config.logHandler != null && !PolyglotLoggers.haveSameTarget(this.config.logHandler, engine.logHandler)) {
this.config.logHandler.close();
}
Expand Down Expand Up @@ -3479,9 +3473,7 @@ boolean patch(PolyglotContextConfig newConfig) {
}
this.config = newConfig;
threadLocalActions.onContextPatch();
if (!newConfig.logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(this, newConfig.logLevels, getAllLoggers());
}
EngineAccessor.LANGUAGE.configureLoggers(this, newConfig.logLevels, getAllLoggers());
final Object[] prev = engine.enter(this);
try {
for (int i = 0; i < this.contexts.length; i++) {
Expand Down Expand Up @@ -3621,9 +3613,7 @@ static PolyglotContextImpl preinitialize(final PolyglotEngineImpl engine, final
context.threadLocalActions.prepareContextStore();
((PreInitializeContextFileSystem) fileSystemConfig.fileSystem).onPreInitializeContextEnd();
((PreInitializeContextFileSystem) fileSystemConfig.internalFileSystem).onPreInitializeContextEnd();
if (!config.logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(context, null, context.getAllLoggers());
}
EngineAccessor.LANGUAGE.configureLoggers(context, null, context.getAllLoggers());
}
}

Expand All @@ -3634,9 +3624,7 @@ Object getOrCreateContextLoggers() {
res = contextBoundLoggers;
if (res == null) {
res = LANGUAGE.createEngineLoggers(PolyglotLoggers.LoggerCache.newContextLoggerCache(this));
if (!this.config.logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(this, this.config.logLevels, res);
}
EngineAccessor.LANGUAGE.configureLoggers(this, this.config.logLevels, res);
contextBoundLoggers = res;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -612,9 +612,7 @@ TruffleLogger getEngineLogger() {
Object logger = EngineAccessor.LANGUAGE.getLoggerCache(result);
LoggerCache loggerCache = (LoggerCache) EngineAccessor.LANGUAGE.getLoggersSPI(logger);
loggerCache.setOwner(this);
if (!logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(this, logLevels, logger);
}
EngineAccessor.LANGUAGE.configureLoggers(this, logLevels, logger);
this.engineLogger = result;
}
}
Expand All @@ -629,14 +627,11 @@ Object getOrCreateEngineLoggers() {
res = engineLoggers;
if (res == null) {
LoggerCache loggerCache = PolyglotLoggers.LoggerCache.newEngineLoggerCache(this);
loggerCache.setOwner(this);
res = LANGUAGE.createEngineLoggers(loggerCache);
if (!logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(this, logLevels, res);
}
EngineAccessor.LANGUAGE.configureLoggers(this, logLevels, res);
for (ContextWeakReference contextRef : contexts) {
PolyglotContextImpl context = contextRef.get();
if (context != null && !context.config.logLevels.isEmpty()) {
if (context != null) {
LANGUAGE.configureLoggers(context, context.config.logLevels, res);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,17 @@ static final class LoggerCache {

private final LogHandler handler;
private final boolean useCurrentContext;
private final Map<String, Level> ownerLogLevels;
private final Function<VMObject, Map<String, Level>> ownerLogLevelsProvider;
private final Set<String> rawLoggerIds;
private final Set<Level> implicitLevels;
private volatile WeakReference<VMObject> ownerRef;

private LoggerCache(LogHandler handler, boolean useCurrentContext, Map<String, Level> ownerLogLevels,
private LoggerCache(LogHandler handler, boolean useCurrentContext, Function<VMObject, Map<String, Level>> ownerLogLevelsProvider,
Set<String> rawLoggerIds, Level... implicitLevels) {
Objects.requireNonNull(handler);
this.handler = handler;
this.useCurrentContext = useCurrentContext;
this.ownerLogLevels = ownerLogLevels;
this.ownerLogLevelsProvider = ownerLogLevelsProvider;
this.rawLoggerIds = rawLoggerIds;
if (implicitLevels.length == 0) {
this.implicitLevels = Collections.emptySet();
Expand All @@ -229,17 +229,19 @@ void setOwner(VMObject owner) {
}

static LoggerCache newEngineLoggerCache(PolyglotEngineImpl engine) {
return newEngineLoggerCache(new PolyglotLogHandler(engine), engine.logLevels, true, Collections.emptySet());
Objects.requireNonNull(engine);
LoggerCache cache = new LoggerCache(new PolyglotLogHandler(engine), true, (owner) -> ((PolyglotEngineImpl) owner).logLevels, Collections.emptySet());
cache.setOwner(engine);
return cache;
}

static LoggerCache newEngineLoggerCache(LogHandler handler, Map<String, Level> logLevels, boolean useCurrentContext,
Set<String> rawLoggerIds, Level... implicitLevels) {
return new LoggerCache(handler, useCurrentContext, logLevels, rawLoggerIds, implicitLevels);
static LoggerCache newEngineLoggerCache(LogHandler handler, Map<String, Level> logLevels, Set<String> rawLoggerIds, Level... implicitLevels) {
return new LoggerCache(handler, false, (owner) -> logLevels, rawLoggerIds, implicitLevels);
}

static LoggerCache newContextLoggerCache(PolyglotContextImpl context) {
Objects.requireNonNull(context);
LoggerCache cache = new LoggerCache(new ContextLogHandler(context), false, context.config.logLevels, Collections.emptySet());
LoggerCache cache = new LoggerCache(new ContextLogHandler(context), false, (owner) -> ((PolyglotContextImpl) owner).config.logLevels, Collections.emptySet());
cache.setOwner(context);
return cache;
}
Expand All @@ -259,13 +261,20 @@ public Map<String, Level> getLogLevels() {
return context.config.logLevels;
}
}
if (ownerLogLevels != null) {
if (ownerRef != null && ownerRef.get() == null) {
// if the owner was initialized and owner was collected we shared the truffle
// logger too far.
throw ContextLogHandler.invalidSharing();
if (ownerLogLevelsProvider != null) {
VMObject owner;
if (ownerRef != null) {
owner = ownerRef.get();
if (owner == null) {
// if the owner was initialized and owner was collected we shared the
// truffle
// logger too far.
throw ContextLogHandler.invalidSharing();
}
} else {
owner = null;
}
return ownerLogLevels;
return ownerLogLevelsProvider.apply(owner);
}
return null;
}
Expand Down Expand Up @@ -773,7 +782,7 @@ public TruffleLogger apply(String loggerId) {
synchronized (this) {
loggersCache = loggers;
if (loggersCache == null) {
LoggerCache spi = LoggerCache.newEngineLoggerCache(logHandler, logLevels, false, Collections.singleton(GRAAL_COMPILER_LOG_ID), Level.INFO);
LoggerCache spi = LoggerCache.newEngineLoggerCache(logHandler, logLevels, Collections.singleton(GRAAL_COMPILER_LOG_ID), Level.INFO);
loggers = loggersCache = EngineAccessor.LANGUAGE.createEngineLoggers(spi);
}
}
Expand Down

0 comments on commit 5d65e5a

Please sign in to comment.