CC: Tweaked

CC: Tweaked

42M Downloads

High resources consumption(?) leading to computer malfunction

sashafiesta opened this issue · 3 comments

commented

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

commented

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.

commented

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 cc-tweaked/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!

commented

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.