Some fixes and improvements to userdata handling.

- Ensure to check for userdata in tables.
- Using local table to track userdata by proxy, allowing the use of metatables reducing the memory consumption.
- Ensure to re-use proxies to avoid duplication across save/load and proper behavior for `==`.

src/main/resources/assets/opencomputers/lua/kernel.lua

@@ -240,10 +240,30 @@ sandbox._G = sandbox
 -------------------------------------------------------------------------------
 -- Start of non-standard stuff.
 
-local wrapUserdata
+-- JNLua derps when the metatable of userdata is changed, so we have to
+-- wrap and isolate it, to make sure it can't be touched by user code.
+-- These functions provide the logic for wrapping and unwrapping (when
+-- pushed to user code and when pushed back to the host, respectively).
+local wrapUserdata, wrapSingleUserdata, unwrapUserdata
+local wrappedUserdata = setmetatable({}, {
+  -- Weak keys, clean up once a proxy is no longer referenced anywhere.
+  __mode="k",
+  -- We need custom persist logic here to avoid ERIS trying to save the
+  -- userdata referenced in this table directly. It will be repopulated
+  -- in the load methods of the persisted userdata wrappers (see below).
+  __persist = function()
+    return function() return {} end
+  end
+})
+
+local function processArguments(...)
+  local args = table.pack(...)
+  unwrapUserdata(args)
+  return table.unpack(args)
+end
 
 local function processResult(result)
-  wrapUserdata(result)
+  wrapUserdata(result) -- needed for metamethods.
   if not result[1] then -- error that should be re-thrown.
     error(result[2], 0)
   else -- success or already processed error.
@@ -261,6 +281,7 @@ local function invoke(target, direct, ...)
   end
   if not result then
     local args = table.pack(...) -- for access in closure
+    unwrapUserdata(args)
     result = select(1, coroutine.yield(function()
       return table.pack(target.invoke(table.unpack(args, 1, args.n)))
     end))
@@ -268,78 +289,108 @@ local function invoke(target, direct, ...)
   return processResult(result)
 end
 
-function wrapUserdata(values)
+-- Metatable for additional functionality on userdata.
-  -- JNLua derps when the metatable of userdata is changed, so we have to
+local userdataWrapper = {
-  -- wrap and isolate it, to make sure it can't be touched by user code.
+  __index = function(self, ...)
-  -- This sadly means we need a separate metatable for each userdata value,
+    return processResult(table.pack(userdata.apply(wrappedUserdata[self], processArguments(...))))
-  -- which means higher memory consumption.
+  end,
-  local function wrap(data)
+  __newindex = function(self, ...)
-    local proxy = {type = "userdata"}
+    return processResult(table.pack(userdata.unapply(wrappedUserdata[self], processArguments(...))))
-    local methods, reason = spcall(userdata.methods, data)
+  end,
-    if not methods then
+  __call = function(self, ...)
-      return nil, reason
+    return processResult(table.pack(userdata.call(wrappedUserdata[self], processArguments(...))))
+  end,
+  __gc = function(self)
+    userdata.dispose(wrappedUserdata[self])
+  end,
+  -- This is the persistence protocol for userdata. Userdata is considered
+  -- to be 'owned' by Lua, and is saved to an NBT tag. We also get the name
+  -- of the actual class when saving, so we can create a new instance via
+  -- reflection when loading again (and then immediately wrap it again).
+  -- Collect wrapped callback methods.
+  __persist = function(self)
+    local className, nbt = userdata.save(wrappedUserdata[self])
+    -- The returned closure is what actually gets persisted, including the
+    -- upvalues, that being the classname and a byte array representing the
+    -- nbt data of the userdata value.
+    return function()
+      return wrapSingleUserdata(userdata.load(className, nbt))
     end
-    do
+  end,
-      local userdataCallback = {
+  -- Do not allow changing the metatable to avoid the gc callback being
-        __call = function(self, ...)
+  -- unset, leading to potential resource leakage on the host side.
-          local methods, reason = spcall(userdata.methods, data)
+  __metatable = "userdata",
-          if not methods then
+  __tostring = "userdata"
-            return nil, reason
+}
-          end
+
-          for name, direct in pairs(methods) do
+local userdataCallback = {
-            if name == self.name then
+  __call = function(self, ...)
-              return invoke(userdata, direct, data, name, ...)
+    local methods = spcall(userdata.methods, wrappedUserdata[self.proxy])
-            end
+    for name, direct in pairs(methods) do
-          end
+      if name == self.name then
-          error("no such method", 1)
+        return invoke(userdata, direct, wrappedUserdata[self.proxy], name, ...)
-        end,
-        __tostring = function(self)
-          return userdata.doc(data, self.name) or "function"
-        end
-      }
-      for method in pairs(methods) do
-        proxy[method] = setmetatable({name=method}, userdataCallback)
       end
     end
-    -- Metatable for additional functionality on userdata.
+    error("no such method", 1)
-    return setmetatable(proxy, {
+  end,
-      __index = function(_, ...)
+  __tostring = function(self)
-        return processResult(table.pack(userdata.apply(data, ...)))
+    return userdata.doc(wrappedUserdata[self.proxy], self.name) or "function"
-      end,
-      __newindex = function(_, ...)
-        return processResult(table.pack(userdata.unapply(data, ...)))
-      end,
-      __call = function(_, ...)
-        return processResult(table.pack(userdata.call(data, ...)))
-      end,
-      __gc = function()
-        userdata.dispose(data)
-      end,
-      -- This is the persistence protocol for userdata. Userdata is considered
-      -- to be 'owned' by Lua, and is saved to an NBT tag. We also get the name
-      -- of the actual class when saving, so we can create a new instance via
-      -- reflection when loading again (and then immediately wrap it again).
-      -- Collect wrapped callback methods.
-      __persist = function()
-        local className, nbt = userdata.save(data)
-        -- The returned closure is what actually gets persisted, including the
-        -- upvalues, that being the classname and a byte array representing the
-        -- nbt data of the userdata value.
-        return function()
-          return wrap(userdata.load(className, nbt))
-        end
-      end,
-      -- Do not allow changing the metatable to avoid the gc callback being
-      -- unset, leading to potential resource leakage on the host side.
-      __metatable = "userdata",
-      __tostring = "userdata"
-    })
   end
-  for i = 1, values.n do
+}
-    if type(values[i]) == "userdata" then
+
-      values[i] = wrap(values[i])
+function wrapSingleUserdata(data)
+  -- Reuse proxies for lower memory consumption and more logical behavior
+  -- without the need of metamethods like __eq, as well as proper reference
+  -- behavior after saving and loading again.
+  for k, v in pairs(wrappedUserdata) do
+    if v == data then
+      return k
     end
   end
+  local proxy = {type = "userdata"}
+  local methods = spcall(userdata.methods, data)
+  for method in pairs(methods) do
+    proxy[method] = setmetatable({name=method, proxy=proxy}, userdataCallback)
+  end
+  wrappedUserdata[proxy] = data
+  return setmetatable(proxy, userdataWrapper)
+end
+
+function wrapUserdata(values)
+  local processed = {}
+  local function wrapRecursively(value)
+    if type(value) == "table" then
+      if not processed[value] then
+        processed[value] = true
+        for k, v in pairs(value) do
+          value[k] = wrapRecursively(v)
+        end
+      end
+    elseif type(value) == "userdata" then
+      return wrapSingleUserdata(value)
+    end
+    return value
+  end
+  wrapRecursively(values)
+end
+
+function unwrapUserdata(values)
+  local processed = {}
+  local function unwrapRecursively(value)
+    if wrappedUserdata[value] then
+      return wrappedUserdata[value]
+    end
+    if type(value) == "table" then
+      if not processed[value] then
+        processed[value] = true
+        for k, v in pairs(value) do
+          value[k] = unwrapRecursively(v)
+        end
+      end
+    end
+    return value
+  end
+  unwrapRecursively(values)
 end
 
 -------------------------------------------------------------------------------
@@ -551,6 +602,7 @@ local function main()
     elseif coroutine.status(co) == "dead" then
       error("computer stopped unexpectedly", 0)
     else
+      unwrapUserdata(result[2])
       args = table.pack(coroutine.yield(result[2])) -- system yielded value
       wrapUserdata(args)
     end

src/main/scala/li/cil/oc/server/component/GraphicsCard.scala

@@ -5,7 +5,6 @@ import li.cil.oc.api.Network
 import li.cil.oc.api.component.TextBuffer.ColorDepth
 import li.cil.oc.api.network._
 import li.cil.oc.common.component.ManagedComponent
-import li.cil.oc.common.tileentity
 import li.cil.oc.util.PackedColor
 import net.minecraft.nbt.NBTTagCompound
 import net.minecraft.util.StatCollector

src/main/scala/li/cil/oc/server/component/machine/NativeLuaArchitecture.scala

@@ -390,7 +390,7 @@ class NativeLuaArchitecture(val machine: api.machine.Machine) extends Architectu
       nbt.setInteger("kernelMemory", math.ceil(kernelMemory / ramScale).toInt)
     } catch {
       case e: LuaRuntimeException =>
-        OpenComputers.log.warning("Could not persist computer.\n" + e.toString + "\tat " + e.getLuaStackTrace.mkString("\n\tat "))
+        OpenComputers.log.warning("Could not persist computer.\n" + e.toString + (if (e.getLuaStackTrace.isEmpty) "" else "\tat " + e.getLuaStackTrace.mkString("\n\tat ")))
         nbt.removeTag("state")
     }
 

src/main/scala/li/cil/oc/server/component/machine/luac/UserdataAPI.scala

@@ -28,16 +28,23 @@ class UserdataAPI(owner: NativeLuaArchitecture) extends NativeLuaAPI(owner) {
     lua.setField(-2, "save")
 
     lua.pushScalaFunction(lua => {
-      val className = lua.toString(1)
+      try {
-      val clazz = Class.forName(className)
+        val className = lua.toString(1)
-      val persistable = clazz.newInstance.asInstanceOf[Persistable]
+        val clazz = Class.forName(className)
-      val data = lua.toByteArray(2)
+        val persistable = clazz.newInstance.asInstanceOf[Persistable]
-      val bais = new ByteArrayInputStream(data)
+        val data = lua.toByteArray(2)
-      val dis = new DataInputStream(bais)
+        val bais = new ByteArrayInputStream(data)
-      val nbt = CompressedStreamTools.read(dis)
+        val dis = new DataInputStream(bais)
-      persistable.load(nbt)
+        val nbt = CompressedStreamTools.read(dis)
-      lua.pushJavaObjectRaw(persistable)
+        persistable.load(nbt)
-      1
+        lua.pushJavaObjectRaw(persistable)
+        1
+      }
+      catch {
+        case t: Throwable =>
+          OpenComputers.log.log(Level.WARNING, "Error in userdata load function.", t)
+          throw t
+      }
     })
     lua.setField(-2, "load")