-
Notifications
You must be signed in to change notification settings - Fork 324
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
System.exit does proper context hard exit. #10363
Conversation
From cmd line.
engine/runtime/src/main/java/org/enso/interpreter/runtime/system/System.java
Outdated
Show resolved
Hide resolved
…em/System.java Co-authored-by: Radosław Waśko <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The
ExitException
should implement getExceptionExitStatus - I'd like to understand what is the benefit of using
ctx.exit
over (improved)ExitException
engine/runtime/src/main/java/org/enso/interpreter/runtime/system/System.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/service/error/ExitException.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe ProgramExcecutionSupport
shouldn't depend on the instanceof
to detect "exit exception". Best to make the ExitException
package private to enforce that.
engine/runtime/src/main/java/org/enso/interpreter/service/error/ExitException.java
Outdated
Show resolved
Hide resolved
...ment-common/src/main/scala/org/enso/interpreter/instrument/job/ProgramExecutionSupport.scala
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/service/error/ExitException.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/service/error/ExitException.java
Outdated
Show resolved
Hide resolved
@@ -880,8 +880,12 @@ private void runMain( | |||
} | |||
} | |||
} catch (PolyglotException e) { | |||
printPolyglotException(e, rootPkgPath); | |||
throw exitFail(); | |||
if (e.isExit()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly we might close the context here to notify other language runtimes that the shutdown is happening... not very urgent.
Also make ExitException package-private.
Fixes #9872
Pull Request Description
The
System.exit 42
component is treated the same way as any other Panic error - it does not interfere with other component evaluation:After removing the
System.exit 42
component, the workflow works as expected. I have also tried opening the project with the component and then removing it.Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
System.exit
in a workflow in IDE and make sure it is usable after that.