Get rid of usages of __gc. Closes #1606.

Wrapping file handles with userdata that has a `dispose` closing the handle.
Internet card already closed via userdata, so nothing to do there.
Removed cleanup logic from term, deal with it.
Added setting to re-enable it, if you really really need it.

# Conflicts:
#	src/main/resources/assets/opencomputers/loot/OpenOS/lib/term.lua
#	src/main/scala/li/cil/oc/server/component/FileSystem.scala

Author: fnuecke

src/main/resources/application.conf

@@ -210,6 +210,12 @@ opencomputers {
       # know what you're doing.
       allowBytecode: false
 
+      # Whether to allow user defined __gc callbacks, i.e. __gc callbacks
+      # defined *inside* the sandbox. Since garbage collection callbacks
+      # are not sandboxed (hooks are disabled while they run), this is not
+      # recommended.
+      allowGC: false
+
       # Whether to make the Lua 5.3 architecture available. If enabled, you
       # can reconfigure any CPU to use the Lua 5.3 architecture.
       enableLua53: true

src/main/resources/assets/opencomputers/loot/OpenOS/bin/sh.lua

@@ -65,7 +65,6 @@ function memoryStream.new()
   local stream = {closed = false, buffer = "",
                   redirect = {}, result = {}, args = {}}
   local metatable = {__index = memoryStream,
-                     __gc = memoryStream.close,
                      __metatable = "memorystream"}
   return setmetatable(stream, metatable)
 end

src/main/resources/assets/opencomputers/loot/OpenOS/lib/filesystem.lua

@@ -509,13 +509,7 @@ function filesystem.open(path, mode)
   end
 
   local stream = {fs = node.fs, handle = handle}
-
-  local function cleanup(self)
-    if not self.handle then return end
-    pcall(self.fs.close, self.handle)
-  end
   local metatable = {__index = fileStream,
-                     __gc = cleanup,
                      __metatable = "filestream"}
   return setmetatable(stream, metatable)
 end

src/main/resources/assets/opencomputers/loot/Plan9k/lib/internet.lua

@@ -98,15 +98,7 @@ function internet.socket(address, port)
   end
 
   local stream = {inet = inet, socket = socket}
-
-  -- stream:close does a syscall, which yields, and that's not possible in
-  -- the __gc metamethod. So we start a timer to do the yield/cleanup.
-  local function cleanup(self)
-    if not self.socket then return end
-    pcall(self.socket.close)
-  end
   local metatable = {__index = socketStream,
-                     __gc = cleanup,
                      __metatable = "socketstream"}
   return setmetatable(stream, metatable)
 end

src/main/resources/assets/opencomputers/loot/Plan9k/lib/modules/base/05_vfs.lua

@@ -528,13 +528,7 @@ function filesystem.open(path, mode)
   end
 
   local stream = {fs = node.fs, handle = handle}
-
-  local function cleanup(self)
-    if not self.handle then return end
-    pcall(self.fs.close, self.handle)
-  end
   local metatable = {__index = fileStream,
-                     __gc = cleanup,
                      __metatable = "filestream"}
   return setmetatable(stream, metatable)
 end

src/main/resources/assets/opencomputers/lua/component/internet/lib/internet.lua

@@ -99,15 +99,7 @@ function internet.socket(address, port)
   end
 
   local stream = {inet = inet, socket = socket}
-
-  -- stream:close does a syscall, which yields, and that's not possible in
-  -- the __gc metamethod. So we start a timer to do the yield/cleanup.
-  local function cleanup(self)
-    if not self.socket then return end
-    pcall(self.socket.close)
-  end
   local metatable = {__index = socketStream,
-                     __gc = cleanup,
                      __metatable = "socketstream"}
   return setmetatable(stream, metatable)
 end

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

@@ -724,7 +724,14 @@ sandbox = {
         sbmt[k] = v
       end
       sbmt.mt = mt
-      sbmt.__gc = sgc
+      -- Garbage collector callbacks apparently can't be sandboxed after
+      -- all, because hooks are disabled while they're running. So we just
+      -- disable them altogether by default.
+      if not system.allowGC() then
+        sbmt.__gc = nil -- Silent fail for backwards compat. TODO error in OC 1.7
+      else
+        sbmt.__gc = sgc
+      end
       mt = sbmt
     end
     return setmetatable(t, mt)

src/main/scala/li/cil/oc/Settings.scala

@@ -76,6 +76,7 @@ class Settings(val config: Config) {
 
   // computer.lua
   val allowBytecode = config.getBoolean("computer.lua.allowBytecode")
+  val allowGC = config.getBoolean("computer.lua.allowGC")
   val enableLua53 = config.getBoolean("computer.lua.enableLua53")
   val ramSizes = Array(config.getIntList("computer.lua.ramSizes"): _*) match {
     case Array(tier1, tier2, tier3, tier4, tier5, tier6) =>

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

@@ -14,6 +14,7 @@ import li.cil.oc.api.machine.Callback
 import li.cil.oc.api.machine.Context
 import li.cil.oc.api.network._
 import li.cil.oc.api.prefab
+import li.cil.oc.api.prefab.AbstractValue
 import li.cil.oc.server.{PacketSender => ServerPacketSender}
 import li.cil.oc.util.ExtendedNBT._
 import net.minecraft.nbt.NBTTagCompound
@@ -122,21 +123,13 @@ class FileSystem(val fileSystem: IFileSystem, var label: Label, val host: Option
     result(success)
   }
 
-  @Callback(direct = true, doc = """function(handle:number) -- Closes an open file descriptor with the specified handle.""")
+  @Callback(direct = true, doc = """function(handle:userdata) -- Closes an open file descriptor with the specified handle.""")
   def close(context: Context, args: Arguments): Array[AnyRef] = fileSystem.synchronized {
-    val handle = args.checkInteger(0)
-    Option(fileSystem.getHandle(handle)) match {
-      case Some(file) =>
-        owners.get(context.node.address) match {
-          case Some(set) if set.remove(handle) => file.close()
-          case _ => throw new IOException("bad file descriptor")
-        }
-      case _ => throw new IOException("bad file descriptor")
-    }
+    close(context, checkHandle(args, 0))
     null
   }
 
-  @Callback(direct = true, limit = 4, doc = """function(path:string[, mode:string='r']):number -- Opens a new file descriptor and returns its handle.""")
+  @Callback(direct = true, limit = 4, doc = """function(path:string[, mode:string='r']):userdata -- Opens a new file descriptor and returns its handle.""")
   def open(context: Context, args: Arguments): Array[AnyRef] = fileSystem.synchronized {
     if (owners.get(context.node.address).fold(false)(_.size >= Settings.get.maxHandles)) {
       throw new IOException("too many open handles")
@@ -148,11 +141,11 @@ class FileSystem(val fileSystem: IFileSystem, var label: Label, val host: Option
       owners.getOrElseUpdate(context.node.address, mutable.Set.empty[Int]) += handle
     }
     diskActivity()
-    result(handle)
+    result(new HandleValue(node.address, handle))
   }
 
   def read(context: Context, args: Arguments): Array[AnyRef] = fileSystem.synchronized {
-    val handle = args.checkInteger(0)
+    val handle = checkHandle(args, 0)
     val n = math.min(Settings.get.maxReadBuffer, math.max(0, args.checkInteger(1)))
     checkOwner(context.node.address, handle)
     Option(fileSystem.getHandle(handle)) match {
@@ -183,7 +176,7 @@ class FileSystem(val fileSystem: IFileSystem, var label: Label, val host: Option
   }
 
   def seek(context: Context, args: Arguments): Array[AnyRef] = fileSystem.synchronized {
-    val handle = args.checkInteger(0)
+    val handle = checkHandle(args, 0)
     val whence = args.checkString(1)
     val offset = args.checkInteger(2)
     checkOwner(context.node.address, handle)
@@ -201,7 +194,7 @@ class FileSystem(val fileSystem: IFileSystem, var label: Label, val host: Option
   }
 
   def write(context: Context, args: Arguments): Array[AnyRef] = fileSystem.synchronized {
-    val handle = args.checkInteger(0)
+    val handle = checkHandle(args, 0)
     val value = args.checkByteArray(1)
     if (!node.tryChangeBuffer(-Settings.get.hddWriteCost * value.length)) {
       throw new IOException("not enough energy")
@@ -218,6 +211,28 @@ class FileSystem(val fileSystem: IFileSystem, var label: Label, val host: Option
 
   // ----------------------------------------------------------------------- //
 
+  def checkHandle(args: Arguments, index: Int) = {
+    if (args.isInteger(index)) {
+      args.checkInteger(index)
+    } else args.checkAny(0) match {
+      case handle: HandleValue => handle.handle
+      case _ => throw new IOException("bad file descriptor")
+    }
+  }
+
+  def close(context: Context, handle: Int): Unit = {
+    Option(fileSystem.getHandle(handle)) match {
+      case Some(file) =>
+        owners.get(context.node.address) match {
+          case Some(set) if set.remove(handle) => file.close()
+          case _ => throw new IOException("bad file descriptor")
+        }
+      case _ => throw new IOException("bad file descriptor")
+    }
+  }
+
+  // ----------------------------------------------------------------------- //
+
   override def onMessage(message: Message) = fileSystem.synchronized {
     super.onMessage(message)
     if (message.name == "computer.stopped" || message.name == "computer.started") {
@@ -317,64 +332,103 @@ object FileSystem {
   // I really need to come up with a way to make the call limit dynamic...
   def apply(fileSystem: IFileSystem, label: Label, host: Option[EnvironmentHost], sound: Option[String], speed: Int = 1): FileSystem = speed match {
     case 6 => new FileSystem(fileSystem, label, host, sound) {
-      @Callback(direct = true, limit = 15, doc = """function(handle:number, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
+      @Callback(direct = true, limit = 15, doc = """function(handle:userdata, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
       override def read(context: Context, args: Arguments): Array[AnyRef] = super.read(context, args)
 
-      @Callback(direct = true, limit = 15, doc = """function(handle:number, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
+      @Callback(direct = true, limit = 15, doc = """function(handle:userdata, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
       override def seek(context: Context, args: Arguments): Array[AnyRef] = super.seek(context, args)
 
-      @Callback(direct = true, limit = 6, doc = """function(handle:number, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
+      @Callback(direct = true, limit = 6, doc = """function(handle:userdata, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
       override def write(context: Context, args: Arguments): Array[AnyRef] = super.write(context, args)
     }
     case 5 => new FileSystem(fileSystem, label, host, sound) {
-      @Callback(direct = true, limit = 13, doc = """function(handle:number, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
+      @Callback(direct = true, limit = 13, doc = """function(handle:userdata, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
       override def read(context: Context, args: Arguments): Array[AnyRef] = super.read(context, args)
 
-      @Callback(direct = true, limit = 13, doc = """function(handle:number, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
+      @Callback(direct = true, limit = 13, doc = """function(handle:userdata, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
       override def seek(context: Context, args: Arguments): Array[AnyRef] = super.seek(context, args)
 
-      @Callback(direct = true, limit = 5, doc = """function(handle:number, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
+      @Callback(direct = true, limit = 5, doc = """function(handle:userdata, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
       override def write(context: Context, args: Arguments): Array[AnyRef] = super.write(context, args)
     }
     case 4 => new FileSystem(fileSystem, label, host, sound) {
-      @Callback(direct = true, limit = 10, doc = """function(handle:number, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
+      @Callback(direct = true, limit = 10, doc = """function(handle:userdata, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
       override def read(context: Context, args: Arguments): Array[AnyRef] = super.read(context, args)
 
-      @Callback(direct = true, limit = 10, doc = """function(handle:number, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
+      @Callback(direct = true, limit = 10, doc = """function(handle:userdata, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
       override def seek(context: Context, args: Arguments): Array[AnyRef] = super.seek(context, args)
 
-      @Callback(direct = true, limit = 4, doc = """function(handle:number, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
+      @Callback(direct = true, limit = 4, doc = """function(handle:userdata, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
       override def write(context: Context, args: Arguments): Array[AnyRef] = super.write(context, args)
     }
     case 3 => new FileSystem(fileSystem, label, host, sound) {
-      @Callback(direct = true, limit = 7, doc = """function(handle:number, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
+      @Callback(direct = true, limit = 7, doc = """function(handle:userdata, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
       override def read(context: Context, args: Arguments): Array[AnyRef] = super.read(context, args)
 
-      @Callback(direct = true, limit = 7, doc = """function(handle:number, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
+      @Callback(direct = true, limit = 7, doc = """function(handle:userdata, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
       override def seek(context: Context, args: Arguments): Array[AnyRef] = super.seek(context, args)
 
-      @Callback(direct = true, limit = 3, doc = """function(handle:number, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
+      @Callback(direct = true, limit = 3, doc = """function(handle:userdata, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
       override def write(context: Context, args: Arguments): Array[AnyRef] = super.write(context, args)
     }
     case 2 => new FileSystem(fileSystem, label, host, sound) {
-      @Callback(direct = true, limit = 4, doc = """function(handle:number, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
+      @Callback(direct = true, limit = 4, doc = """function(handle:userdata, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
       override def read(context: Context, args: Arguments): Array[AnyRef] = super.read(context, args)
 
-      @Callback(direct = true, limit = 4, doc = """function(handle:number, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
+      @Callback(direct = true, limit = 4, doc = """function(handle:userdata, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
       override def seek(context: Context, args: Arguments): Array[AnyRef] = super.seek(context, args)
 
-      @Callback(direct = true, limit = 2, doc = """function(handle:number, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
+      @Callback(direct = true, limit = 2, doc = """function(handle:userdata, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
       override def write(context: Context, args: Arguments): Array[AnyRef] = super.write(context, args)
     }
     case _ => new FileSystem(fileSystem, label, host, sound) {
-      @Callback(direct = true, limit = 1, doc = """function(handle:number, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
+      @Callback(direct = true, limit = 1, doc = """function(handle:userdata, count:number):string or nil -- Reads up to the specified amount of data from an open file descriptor with the specified handle. Returns nil when EOF is reached.""")
       override def read(context: Context, args: Arguments): Array[AnyRef] = super.read(context, args)
 
-      @Callback(direct = true, limit = 1, doc = """function(handle:number, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
+      @Callback(direct = true, limit = 1, doc = """function(handle:userdata, whence:string, offset:number):number -- Seeks in an open file descriptor with the specified handle. Returns the new pointer position.""")
       override def seek(context: Context, args: Arguments): Array[AnyRef] = super.seek(context, args)
 
-      @Callback(direct = true, limit = 1, doc = """function(handle:number, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
+      @Callback(direct = true, limit = 1, doc = """function(handle:userdata, value:string):boolean -- Writes the specified data to an open file descriptor with the specified handle.""")
       override def write(context: Context, args: Arguments): Array[AnyRef] = super.write(context, args)
     }
   }
 }
+
+final class HandleValue extends AbstractValue {
+  def this(owner: String, handle: Int) = {
+    this()
+    this.owner = owner
+    this.handle = handle
+  }
+
+  var owner = ""
+  var handle = 0
+
+  override def dispose(context: Context): Unit = {
+    super.dispose(context)
+    if (context.node() != null && context.node().network() != null) {
+      val node = context.node().network().node(owner)
+      if (node != null) {
+        node.host() match {
+          case fs: FileSystem => try fs.close(context, handle) catch {
+            case _: Throwable => // Ignore, already closed.
+          }
+        }
+      }
+    }
+  }
+
+  override def load(nbt: NBTTagCompound): Unit = {
+    super.load(nbt)
+    owner = nbt.getString("owner")
+    handle = nbt.getInteger("handle")
+  }
+
+  override def save(nbt: NBTTagCompound): Unit = {
+    super.save(nbt)
+    nbt.setInteger("handle", handle)
+    nbt.setString("owner", owner)
+  }
+
+  override def toString: String = handle.toString
+}

src/main/scala/li/cil/oc/server/machine/luac/SystemAPI.scala

@@ -32,6 +32,13 @@ class SystemAPI(owner: NativeLuaArchitecture) extends NativeLuaAPI(owner) {
     })
     lua.setField(-2, "allowBytecode")
 
+    // Whether custom __gc callbacks are allowed.
+    lua.pushScalaFunction(lua => {
+      lua.pushBoolean(Settings.get.allowGC)
+      1
+    })
+    lua.setField(-2, "allowGC")
+
     // How long programs may run without yielding before we stop them.
     lua.pushScalaFunction(lua => {
       lua.pushNumber(Settings.get.timeout)