Avoid NPE during instrumentation (#8317)

Fixes a random crash (*) during instrumentation. Notice how `onTailCallReturn` calls `onReturnValue` with `null` frame.

Bonus: noticed that for some reason we weren't getting logs for `ExecutionService`. This turned out to be the problem with the logger name which by default was `[enso]` not
`[enso.org.enso.interpreter.service.ExecutionService`] and there is some logic there that normalizes the name and assumed a dot after `enso`. This change fixes the logic.

(*)
```
[enso.org.enso.interpreter.service.ExecutionService] Execution of function main failed (Cannot invoke "com.oracle.truffle.api.frame.VirtualFrame.materialize()" because "frame" is null).
java.lang.NullPointerException: Cannot invoke "com.oracle.truffle.api.frame.VirtualFrame.materialize()" because "frame" is null
at org.enso.interpreter.instrument.IdExecutionInstrument$IdEventNodeFactory$IdExecutionEventNode.onReturnValue(IdExecutionInstrument.java:246)
at org.enso.interpreter.instrument.IdExecutionInstrument$IdEventNodeFactory$IdExecutionEventNode.onTailCallReturn(IdExecutionInstrument.java:274)
at org.enso.interpreter.instrument.IdExecutionInstrument$IdEventNodeFactory$IdExecutionEventNode.onReturnExceptional(IdExecutionInstrument.java:258)
at org.graalvm.truffle/com.oracle.truffle.api.instrumentation.ProbeNode$EventProviderChainNode.innerOnReturnExceptional(ProbeNode.java:1395)
at org.graalvm.truffle/com.oracle.truffle.api.instrumentation.ProbeNode$EventChainNode.onReturnExceptional(ProbeNode.java:1031)
at org.graalvm.truffle/com.oracle.truffle.api.instrumentation.ProbeNode.onReturnExceptionalOrUnwind(ProbeNode.java:296)
at org.enso.interpreter.node.ExpressionNodeWrapper.executeGeneric(ExpressionNodeWrapper.java:119)
at org.enso.interpreter.node.ClosureRootNode.execute(ClosureRootNode.java:85)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.executeRootNode(OptimizedCallTarget.java:718)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.profiledPERoot(OptimizedCallTarget.java:641)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callBoundary(OptimizedCallTarget.java:574)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.doInvoke(OptimizedCallTarget.java:558)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callDirect(OptimizedCallTarget.java:504)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedDirectCallNode.call(OptimizedDirectCallNode.java:69)
at org.enso.interpreter.node.callable.thunk.ThunkExecutorNode.doCached(ThunkExecutorNode.java:69)
at org.enso.interpreter.node.callable.thunk.ThunkExecutorNodeGen.executeAndSpecialize(ThunkExecutorNodeGen.java:207)
at org.enso.interpreter.node.callable.thunk.ThunkExecutorNodeGen.executeThunk(ThunkExecutorNodeGen.java:167)
...
```

# Important Notes
Fixes regressions introduced in #8148 and #8162
This commit is contained in:
Hubert Plociniczak 2023-11-17 15:38:27 +01:00 committed by GitHub
parent aef3a4ffc0
commit 348f5170ab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 69 additions and 5 deletions

View File

@ -72,7 +72,7 @@ public final class ExecutionService {
private final EnsoContext context;
private final Optional<IdExecutionService> idExecutionInstrument;
private final NotificationHandler.Forwarder notificationForwarder;
private final TruffleLogger logger = TruffleLogger.getLogger(LanguageInfo.ID);
private final TruffleLogger logger = TruffleLogger.getLogger(LanguageInfo.ID, ExecutionService.class);
private final ConnectedLockManager connectedLockManager;
private final ExecuteRootNode execute = new ExecuteRootNode();
private final CallRootNode call = new CallRootNode();

View File

@ -234,7 +234,7 @@ public class IdExecutionInstrument extends TruffleInstrument implements IdExecut
functionCallInstrumentationNode.getId(),
result,
nanoTimeElapsed,
frame.materialize(),
frame == null ? null : frame.materialize(),
node);
Object cachedResult = callbacks.onFunctionReturn(info);
if (cachedResult != null) {
@ -243,7 +243,7 @@ public class IdExecutionInstrument extends TruffleInstrument implements IdExecut
} else if (node instanceof ExpressionNode expressionNode) {
Info info =
new NodeInfo(
expressionNode.getId(), result, nanoTimeElapsed, frame.materialize(), node);
expressionNode.getId(), result, nanoTimeElapsed, frame == null ? null : frame.materialize(), node);
callbacks.updateCachedResult(info);
if (info.isPanic()) {

View File

@ -6535,4 +6535,66 @@ class RuntimeServerTest
.name
}
it should "handle tailcalls" in {
val contextId = UUID.randomUUID()
val requestId = UUID.randomUUID()
val moduleName = "Enso_Test.Test.Main"
val metadata = new Metadata
val code =
"""main =
| fac 10
|
|fac n =
| acc n v = if n <= 1 then v else
| @Tail_Call acc n-1 n*v
|
| acc n 1
|""".stripMargin.linesIterator.mkString("\n")
val res = metadata.addItem(11, 6, "aa")
val contents = metadata.appendToCode(code)
val mainFile = context.writeMain(contents)
// create context
context.send(Api.Request(requestId, Api.CreateContextRequest(contextId)))
context.receive shouldEqual Some(
Api.Response(requestId, Api.CreateContextResponse(contextId))
)
// Set sources for the module
context.send(
Api.Request(requestId, Api.OpenFileRequest(mainFile, contents))
)
context.receive shouldEqual Some(
Api.Response(Some(requestId), Api.OpenFileResponse)
)
// push main
context.send(
Api.Request(
requestId,
Api.PushContextRequest(
contextId,
Api.StackItem.ExplicitCall(
Api.MethodPointer(moduleName, moduleName, "main"),
None,
Vector()
)
)
)
)
context.receiveN(3) should contain theSameElementsAs Seq(
Api.Response(requestId, Api.PushContextResponse(contextId)),
TestMessages.update(
contextId,
res,
ConstantsGen.INTEGER_BUILTIN,
methodCall =
Some(Api.MethodCall(Api.MethodPointer(moduleName, moduleName, "fac")))
),
context.executionComplete(contextId)
)
}
}

View File

@ -23,7 +23,7 @@ public class ApplicationFilter extends Filter<ILoggingEvent> {
this.loggers = loggers;
this.level = level;
this.prefix = prefix;
this.prefixLength = prefix != null ? prefix.length() + 1 : 0; // inlude `.` in `enso.`
this.prefixLength = prefix != null ? prefix.length() : 0;
}
@Override
@ -48,7 +48,9 @@ public class ApplicationFilter extends Filter<ILoggingEvent> {
private boolean loggerNameMatches(String validLoggerName, String eventLoggerName) {
if (prefix != null && eventLoggerName.startsWith(prefix)) {
return eventLoggerName.substring(prefixLength).startsWith(validLoggerName);
int normalizedLoggerNameIdx =
eventLoggerName.indexOf(".") == prefixLength ? prefixLength + 1 : prefixLength;
return eventLoggerName.substring(normalizedLoggerNameIdx).startsWith(validLoggerName);
} else {
return eventLoggerName.startsWith(validLoggerName);
}