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

High resources consumption(?) leading to computer malfunction #1333

Closed
sashafiesta opened this issue Feb 15, 2023 · 3 comments
Closed

High resources consumption(?) leading to computer malfunction #1333

sashafiesta opened this issue Feb 15, 2023 · 3 comments
Labels
area-Core This affects CC's core (the Lua runtime, APIs, computer internals). bug A problem or unexpected behaviour with the mod.
Milestone

Comments

@sashafiesta
Copy link

Minecraft Version

1.18.x

Version

1.101.2

Details

If you run the code on several computers (two are enough), then all other computers on the server will stop responding and executing commands.

Also, after a while, the computer running code will display the following message:

Error running computer
Too long without yielding
ComputerCraft may be installed incorrectly

And will stop responding to the player's actions until it is broken and replaced.

The entire code:

print(load(function() return string.rep("\n", 2^24) end))

Link to the logs:
https://gist.github.com/sashafiesta/828d207f2e3ddea874663e19c5065777

It can be abused. You can make a turtle that will constantly place and break a computer with a similar code in startup.
So, if the chunk is loaded, all computers on the server will become inoperable

@sashafiesta sashafiesta added the bug A problem or unexpected behaviour with the mod. label Feb 15, 2023
@Lupus590
Copy link
Contributor

There's not much that can be done about this that hasn't already been done. It was a design choice (and in part a limitation of how the Lua engine is implemented into the mod) to have all computers share the CPU one at a time (or more if the config allows it) with a maximum run of about 10 seconds each turn- if CC didn't have this limitation the bad code you described would have disrupted the tick rate of the entire MC server as more of the server's CPU gets used by bad CC computers.

CC already provides tools to server admins to find bad computers such as the one you described, CC's diagnostics/moderating commands allow them to teleport to bad computers. I assume that most admins seeing the turtle contraption and the code would probably recognise the malicious intent and intervene appropriately.

@SquidDev
Copy link
Member

SquidDev commented Feb 15, 2023

I do actually think there's more that can be done here. The computer is being force-killed, which suggests our normal measures for stopping the computer aren't kicking in.

I did a small amount of profiling here, and we can see that most of the time is split between the call to string.rep and the parser inside of load:

image

As the Lua interpreter itself is rarely active (from what I can tell, the function argument to load is only called ~30 times1), we don't get many opportunities to check if we've timed out, meaning it's possible we miss the check entirely. I think there's a couple of improvements we could/should make here:

  • Ensure the "too long without yielding" check is hit more often. I've had a couple of ideas here that I think would improve performance and make this more robust, so maybe this issue is a good motivation for that :).
  • Limit memory usage/allocations. Really there's no reason people should be allocating 16MiB strings. Stop em! I've filed a more detail issue for this at Memory limits via allocation sampling Cobalt#66.
  • Poll the timeout state inside the Lua parser. We do this inside some of the string pattern matching functions already - maybe we should inside load too?

Footnotes

  1. This function gets compiled to 5 instructions (well, 7, but I'm ignoring the returns). This means when the function is called 30 times, we only run 150 instructions. The timeout poll only happens ever 128 instructions, so the odds of hitting it when the computer should timeout are quite low!

@SquidDev
Copy link
Member

SquidDev commented Feb 15, 2023

As an aside, I wonder if we could optimise string.rep to double the size of the copied portion each time, which would reduce the copies needed from $n$ to $\log_2(n)$. string.rep shouldn't be the hotspot here!

Edit: And done! Fun facts: some JS engines represent repeated strings with a separate object, a bit like we represent concatenation with ropes. I have suppressed the urge to do that.

@SquidDev SquidDev added the area-Core This affects CC's core (the Lua runtime, APIs, computer internals). label Feb 15, 2023
@SquidDev SquidDev added this to the 1.104.0 milestone Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Core This affects CC's core (the Lua runtime, APIs, computer internals). bug A problem or unexpected behaviour with the mod.
Projects
None yet
Development

No branches or pull requests

3 participants