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

Machine.String() hangs the VM on error when printing large structure #1981

Closed
jefft0 opened this issue Apr 25, 2024 · 4 comments · May be fixed by #2385
Closed

Machine.String() hangs the VM on error when printing large structure #1981

jefft0 opened this issue Apr 25, 2024 · 4 comments · May be fixed by #2385
Assignees
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@jefft0
Copy link
Contributor

jefft0 commented Apr 25, 2024

Description

If a transaction fails, for example running out of gas, the VM prints the state with Machine.String() . If the state has a large recursive structure, like an avl.Tree(), this can hang the VM as it constructs the string. Please see the analysis here.

Your environment

  • The latest gno as of 2024-04-25
  • master branch 688edf5

Steps to reproduce

Please see PR #1736 which has a txtar to reproduce the bug and has analysis of the root cause.

Expected behaviour

On error, the VM should print a useful error message.

Actual behaviour

The VM attempts to dump the entire machine state, even if there is a large recursive data structure.

Proposed solution

This comment proposes to "switch from doing Machine.String() to printing a stacktrace. (This is likely to be more useful and parseable information by the programmer anyway.)"

@jefft0 jefft0 changed the title Machine.String() hangs the VM when printing error Machine.String() hangs the VM on error when printing large structure Apr 25, 2024
@Kouteki Kouteki added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related labels Apr 26, 2024
@Kouteki Kouteki moved this from Triage to Backlog in 🧙‍♂️gno.land core team Apr 26, 2024
@petar-dambovaliev petar-dambovaliev self-assigned this Apr 26, 2024
@Kouteki Kouteki moved this from Backlog to Todo in 🧙‍♂️gno.land core team Apr 26, 2024
@petar-dambovaliev
Copy link
Contributor

I agree with the solution from @thehowl. Since he is working on it already, i'll take this opportunity to improve the String() method on the Machine struct because it looks very rugged.

petar-dambovaliev added a commit that referenced this issue May 3, 2024
Make the `String` method on the `Machine` struct faster by preallocating
a string builder.

[issue](#1981)
@thehowl
Copy link
Member

thehowl commented May 7, 2024

@petar-dambovaliev BTW I haven't taken up on it, I troubleshot the issue because I was pointed at it, but the actual implementation of the stacktraces will take quite some work

You're welcome to take up on this if you want to

@jefft0
Copy link
Contributor Author

jefft0 commented May 15, 2024

In a dev call, it was suggested that #1994 could fix this issue. It does not. The test still hangs in the txtar of #1736 .

@jefft0
Copy link
Contributor Author

jefft0 commented Jul 1, 2024

Closing this tracking issue to focus on the PR with the txtar that demonstrates the bug. #1736

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Development

Successfully merging a pull request may close this issue.

4 participants