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

Avoid NPEs during regular execution #7482

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Avoid NPEs during regular execution #7482

merged 2 commits into from
Aug 7, 2023

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented Aug 2, 2023

Pull Request Description

Seeing plenty of

java.lang.NullPointerException: Some(Null receiver values are not supported by libraries.)
  at org.graalvm.truffle/com.oracle.truffle.api.library.LibraryFactory.dispatch(LibraryFactory.java:528)
  at org.graalvm.truffle/com.oracle.truffle.api.library.LibraryFactory.getUncached(LibraryFactory.java:396)
  at org.enso.interpreter.runtime.error.WarningsLibraryGen$UncachedDispatch.hasWarnings(WarningsLibraryGen.java:440)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.sendExpressionUpdate(ProgramExecutionSupport.scala:366)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$executeProgram$1(ProgramExecutionSupport.scala:62)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$executeProgram$10(ProgramExecutionSupport.scala:151)
  at java.base/java.lang.Iterable.forEach(Iterable.java:75)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.executeProgram(ProgramExecutionSupport.scala:139)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$runProgram$3(ProgramExecutionSupport.scala:217)

during execution of simple programs.
Added a guard to prevent us from sending expression updates when dealing with nulls.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.

@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Aug 2, 2023
@JaroslavTulach
Copy link
Member

Interesting. I'd try to avoid calling hasWarnings:

  at org.enso.interpreter.runtime.error.WarningsLibraryGen$UncachedDispatch.hasWarnings(WarningsLibraryGen.java:440)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.sendExpressionUpdate(ProgramExecutionSupport.scala:366)

when the value is null but otherwise I'd continue sending update.

@hubertp
Copy link
Contributor Author

hubertp commented Aug 2, 2023

Interesting. I'd try to avoid calling hasWarnings:

  at org.enso.interpreter.runtime.error.WarningsLibraryGen$UncachedDispatch.hasWarnings(WarningsLibraryGen.java:440)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.sendExpressionUpdate(ProgramExecutionSupport.scala:366)

when the value is null but otherwise I'd continue sending update.

So essentially we want to send an empty payload? I'm not sure how useful that is.

@4e6
Copy link
Contributor

4e6 commented Aug 2, 2023

So essentially we want to send an empty payload? I'm not sure how useful that is.

IDE can be interested in other info, type for example. The null value is quite suspicious and may indicate an error somewhere during the execution. But I'd rather continue sending the updates about the executed node.

Interesting. I'd try to avoid calling hasWarnings

If it helps, I'd try that instead of suppressing the update completely

Seeing plenty of
```
java.lang.NullPointerException: Some(Null receiver values are not supported by libraries.)
  at org.graalvm.truffle/com.oracle.truffle.api.library.LibraryFactory.dispatch(LibraryFactory.java:528)
  at org.graalvm.truffle/com.oracle.truffle.api.library.LibraryFactory.getUncached(LibraryFactory.java:396)
  at org.enso.interpreter.runtime.error.WarningsLibraryGen$UncachedDispatch.hasWarnings(WarningsLibraryGen.java:440)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.sendExpressionUpdate(ProgramExecutionSupport.scala:366)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$executeProgram$1(ProgramExecutionSupport.scala:62)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$executeProgram$10(ProgramExecutionSupport.scala:151)
  at java.base/java.lang.Iterable.forEach(Iterable.java:75)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.executeProgram(ProgramExecutionSupport.scala:139)
  at org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$runProgram$3(ProgramExecutionSupport.scala:217)
```
during execution of simple programs.
Added a guard to prevent us from sending expression updates when dealing
with nulls.
Still want to send an update even if the payload is empty.
@hubertp
Copy link
Contributor Author

hubertp commented Aug 4, 2023

@4e6 @JaroslavTulach updated as requested.
Will potentially help with #7357

@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Aug 7, 2023
@mergify mergify bot merged commit 162debb into develop Aug 7, 2023
25 of 27 checks passed
@mergify mergify bot deleted the wip/hubert/avoid-npe branch August 7, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants