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

feat: add os.Exit #686

Closed
wants to merge 1 commit into from
Closed

feat: add os.Exit #686

wants to merge 1 commit into from

Conversation

albttx
Copy link
Member

@albttx albttx commented Apr 1, 2023

Description

Allow to exit a realm

Happy Easter ! :)
@albttx albttx requested a review from a team as a code owner April 1, 2023 18:18
@piux2
Copy link
Contributor

piux2 commented Apr 1, 2023

Can you describe 1) the problem to solve and 2) the trade-off made with this solution?

We should put extra cautious in creating the native method in stdlibs to alter the state or intercept the execution of the machine.

@albttx
Copy link
Member Author

albttx commented Apr 1, 2023

In case you need to exit in a smart contract

@piux2
Copy link
Contributor

piux2 commented Apr 2, 2023

In case you need to exit in a smart contract

Sure, in what cases do we need to exit a smart contract that can not be handled at the end of the function call in a package managed by GVM runtime?

Calling os.exit() directly from a contract breaks the contract call flow and GVM persistent flow.

@moul
Copy link
Member

moul commented Apr 2, 2023

panic() informs users of what went wrong in the event of a failure. To allow for silent panicking, let's create a type that displays less intimidating stack traces through #416. While developers may prefer exiting gracefully, users want to know what happened if the contract fails, and panic() already satisfies this need. If we improve our panic() handling, I'm open to making so.Exit a panic(silentExit{}) shortcut.

@anarcher
Copy link
Contributor

anarcher commented Apr 2, 2023

IMHO, Inside a VM, it seems like a good idea to define an operation like op.Exit, like os.Exit (don't use it), and approach it as a way to shut down the machine when it encounters a VM.

@thehowl
Copy link
Member

thehowl commented Apr 3, 2023

I also tend to agree that it is not a good idea to add os.Exit to exit the smart contract. On top of this, this should exit the smart contract, not actually call an os.Exit -- because that would kill the program, even if it has goroutines or recover() calls -- so when this is called on gnoland the node would be shut down.

I prefer much more a defer-recover in the functions of the contract that need to be able to exit with a specific error than can be used to exit it.

@albttx
Copy link
Member Author

albttx commented Apr 3, 2023

guys... this was an April fool ... i guess it's failed 😂

@albttx albttx closed this Apr 3, 2023
@thehowl
Copy link
Member

thehowl commented Apr 3, 2023

lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants