MightyPirates / OpenComputers

Home of the OpenComputers mod for Minecraft.
https://oc.cil.li
Other
1.59k stars 431 forks source link

Computers can cause Minecraft to use all available memory when converting objects from Lua to Java #1774

Closed ds84182 closed 3 years ago

ds84182 commented 8 years ago

The Bug

A single computer can cause Minecraft to use all available memory in the Java heap while moving values in component.invoke from Lua to Java. Using all available memory causes Java's "stop the world" GC to run which will pause the server for a measurable amount of time. This may also affect any functions that use varargs from Lua to Java (userdata invocations come to mind).

Example (In "psuedocode")

  1. Create a large string. In my testing I used a 32kb string.
  2. Create a table and set as many elements as possible to a reference to the same large string. In my testing I used a table with 1024*24 entries. You could possibly use a table with a single table that contains more nested tables for maximum effect ({{x,{x,{x}}}...).
  3. Call component.invoke or any other function that unboxes varargs with the table unpacked.

    How to fix (potentially)

    • Option 1: Allow architectures to pass in a custom Argument object to Machine.invoke. Architectures can do specific things inside of their own Argument objects to pass the values to Java in a safe manner. (Pros: More flexibility for architecture authors :+1:; Cons: Changes the API around on the Java side but this option could be combined with Option 2 to make a backwards compatible version)
    • Option 2: Limit the amount of arguments that can be passed into a function to something like 64 arguments. (Pros: Not much code to change; Cons: Something could slip through the cracks and we'd be back at square 1, someone using more than 64 arguments would lose compatibility)
fnuecke commented 8 years ago

Is this LuaC and/or LuaJ? I was pretty sure LuaC had an arg count limit of 255 anyway.

ds84182 commented 8 years ago

My testing was done in LuaC but LuaJ seems like it could have the problem too.

SoniEx2 commented 6 years ago

Isn't this solveable with interning?