Merge pull request #101 from tims-bsquare/feature/various-safety-changes

Various safety changes, bugfixes and added null type

README.md

@@ -186,7 +186,7 @@ npm test # ensure everything it's working fine
 
 ### Null
 
-`null` is not exposed to Lua and it has no awareness of it which can cause some issues when using it a table. `nil` is equivalent to `undefined`. Issue #39 tracks this and a workaround until `null` is added into Wasmoon.
+`null` is injected as userdata type if `injectObjects` is set to `true`. This works as expected except that it will evaluate to `true` in Lua.
 
 ### Promises
 

src/engine.ts

@@ -2,6 +2,7 @@ import Global from './global'
 import Thread from './thread'
 import createErrorType from './type-extensions/error'
 import createFunctionType from './type-extensions/function'
+import createNullType from './type-extensions/null'
 import createPromiseType from './type-extensions/promise'
 import createProxyType from './type-extensions/proxy'
 import createTableType from './type-extensions/table'
@@ -13,17 +14,29 @@ export default class LuaEngine {
 
     public constructor(
         private cmodule: LuaWasm,
-        { openStandardLibs = true, injectObjects = false, enableProxy = true, traceAllocations = false } = {},
+        {
+            openStandardLibs = true,
+            injectObjects = false,
+            enableProxy = true,
+            traceAllocations = false,
+            functionTimeout = undefined as number | undefined,
+        } = {},
     ) {
         this.global = new Global(this.cmodule, traceAllocations)
 
         // Generic handlers - These may be required to be registered for additional types.
         this.global.registerTypeExtension(0, createTableType(this.global))
-        this.global.registerTypeExtension(0, createFunctionType(this.global))
+        this.global.registerTypeExtension(0, createFunctionType(this.global, { functionTimeout }))
 
         // Contains the :await functionality.
         this.global.registerTypeExtension(1, createPromiseType(this.global, injectObjects))
 
+        if (injectObjects) {
+            // Should be higher priority than table since that catches generic objects along
+            // with userdata so it doesn't end up a userdata type.
+            this.global.registerTypeExtension(5, createNullType(this.global))
+        }
+
         if (enableProxy) {
             // This extension only really overrides tables and arrays.
             // When a function is looked up in one of it's tables it's bound and then

src/global.ts

@@ -41,7 +41,12 @@ export default class Global extends Thread {
                 'iiiii',
             )
 
-            super(cmodule, [], cmodule.lua_newstate(allocatorFunctionPointer, null))
+            const address = cmodule.lua_newstate(allocatorFunctionPointer, null)
+            if (!address) {
+                cmodule.module.removeFunction(allocatorFunctionPointer)
+                throw new Error('lua_newstate returned a null pointer')
+            }
+            super(cmodule, [], address)
 
             this.memoryStats = memoryStats
             this.allocatorFunctionPointer = allocatorFunctionPointer

src/thread.ts

@@ -45,7 +45,7 @@ export default class Thread {
         if (!address) {
             throw new Error('lua_newthread returned a null pointer')
         }
-        return new Thread(this.lua, this.typeExtensions, address)
+        return new Thread(this.lua, this.typeExtensions, address, this.parent || this)
     }
 
     public resetThread(): void {
@@ -186,7 +186,7 @@ export default class Thread {
 
     public pushValue(rawValue: unknown, userdata?: unknown): void {
         const decoratedValue = this.getValueDecorations(rawValue)
-        const target = decoratedValue.target ?? undefined
+        const target = decoratedValue.target
 
         if (target instanceof Thread) {
             const isMain = this.lua.lua_pushthread(target.address) === 1

src/type-extensions/function.ts

@@ -18,6 +18,10 @@ export function decorateFunction(target: FunctionType, options: FunctionDecorati
     return new Decoration<FunctionType, FunctionDecoration>(target, options)
 }
 
+export interface FunctionTypeExtensionOptions {
+    functionTimeout?: number
+}
+
 class FunctionTypeExtension extends TypeExtension<FunctionType, FunctionDecoration> {
     private readonly functionRegistry =
         typeof FinalizationRegistry !== 'undefined'
@@ -30,10 +34,21 @@ class FunctionTypeExtension extends TypeExtension<FunctionType, FunctionDecorati
 
     private gcPointer: number
     private functionWrapper: number
+    private callbackContext: Thread
+    private callbackContextIndex: number
+    private options?: FunctionTypeExtensionOptions
 
-    public constructor(thread: Global) {
+    public constructor(thread: Global, options?: FunctionTypeExtensionOptions) {
         super(thread, 'js_function')
 
+        this.options = options
+        // Create a thread off of the global thread to be used to create function call threads without
+        // interfering with the global context. This creates a callback context that will always exist
+        // even if the thread that called getValue() has been destroyed.
+        this.callbackContext = thread.newThread()
+        // Pops it from the global stack but keeps it alive
+        this.callbackContextIndex = this.thread.lua.luaL_ref(thread.address, LUA_REGISTRYINDEX)
+
         if (!this.functionRegistry) {
             console.warn('FunctionTypeExtension: FinalizationRegistry not found. Memory leaks likely.')
         }
@@ -117,6 +132,10 @@ class FunctionTypeExtension extends TypeExtension<FunctionType, FunctionDecorati
     public close(): void {
         this.thread.lua.module.removeFunction(this.gcPointer)
         this.thread.lua.module.removeFunction(this.functionWrapper)
+        // Doesn't destroy the Lua thread, just function pointers.
+        this.callbackContext.close()
+        // Destroy the Lua thread
+        this.callbackContext.lua.luaL_unref(this.callbackContext.address, LUA_REGISTRYINDEX, this.callbackContextIndex)
     }
 
     public isType(_thread: Thread, _index: number, type: LuaType): boolean {
@@ -155,38 +174,58 @@ class FunctionTypeExtension extends TypeExtension<FunctionType, FunctionDecorati
     }
 
     public getValue(thread: Thread, index: number): FunctionType {
+        // Create a copy of the function
         thread.lua.lua_pushvalue(thread.address, index)
+        // Create a reference to the function which pops it from the stack
         const func = thread.lua.luaL_ref(thread.address, LUA_REGISTRYINDEX)
 
         const jsFunc = (...args: any[]): any => {
-            if (thread.isClosed()) {
+            // Calling a function would ideally be in the Lua context that's calling it. For example if the JS function
+            // setInterval were exposed to Lua then the calling thread would be created in that Lua context for executing
+            // the function call back to Lua through JS. However, if getValue were called in a thread, the thread then
+            // destroyed, and then this JS func were called it would be calling from a dead context. That means the safest
+            // thing to do is to have a thread you know will always exist.
+            if (this.callbackContext.isClosed()) {
                 console.warn('Tried to call a function after closing lua state')
                 return
             }
 
-            const internalType = thread.lua.lua_rawgeti(thread.address, LUA_REGISTRYINDEX, BigInt(func))
-            if (internalType !== LuaType.Function) {
-                const callMetafieldType = thread.lua.luaL_getmetafield(thread.address, -1, '__call')
-                thread.pop()
-                if (callMetafieldType !== LuaType.Function) {
-                    throw new Error(`A value of type '${internalType}' was pushed but it is not callable`)
+            // Function calls back to value should always be within a new thread because
+            // they can be left in inconsistent states.
+            const callThread = this.callbackContext.newThread()
+            try {
+                const internalType = callThread.lua.lua_rawgeti(callThread.address, LUA_REGISTRYINDEX, BigInt(func))
+                if (internalType !== LuaType.Function) {
+                    const callMetafieldType = callThread.lua.luaL_getmetafield(callThread.address, -1, '__call')
+                    callThread.pop()
+                    if (callMetafieldType !== LuaType.Function) {
+                        throw new Error(`A value of type '${internalType}' was pushed but it is not callable`)
+                    }
                 }
-            }
 
-            for (const arg of args) {
-                thread.pushValue(arg)
-            }
+                for (const arg of args) {
+                    callThread.pushValue(arg)
+                }
 
-            const status: LuaReturn = thread.lua.lua_pcallk(thread.address, args.length, 1, 0, 0, null)
-            if (status === LuaReturn.Yield) {
-                throw new Error('cannot yield in callbacks from javascript')
-            }
-            thread.assertOk(status)
+                if (this.options?.functionTimeout) {
+                    callThread.setTimeout(Date.now() + this.options.functionTimeout)
+                }
 
-            const result = thread.getValue(-1)
+                const status: LuaReturn = callThread.lua.lua_pcallk(callThread.address, args.length, 1, 0, 0, null)
+                if (status === LuaReturn.Yield) {
+                    throw new Error('cannot yield in callbacks from javascript')
+                }
+                callThread.assertOk(status)
 
-            thread.pop()
-            return result
+                if (callThread.getTop() > 0) {
+                    return callThread.getValue(-1)
+                }
+                return undefined
+            } finally {
+                callThread.close()
+                // Pop thread used for function call.
+                this.callbackContext.pop()
+            }
         }
 
         this.functionRegistry?.register(jsFunc, func)
@@ -195,6 +234,9 @@ class FunctionTypeExtension extends TypeExtension<FunctionType, FunctionDecorati
     }
 }
 
-export default function createTypeExtension(thread: Global): TypeExtension<FunctionType, FunctionDecoration> {
-    return new FunctionTypeExtension(thread)
+export default function createTypeExtension(
+    thread: Global,
+    options?: FunctionTypeExtensionOptions,
+): TypeExtension<FunctionType, FunctionDecoration> {
+    return new FunctionTypeExtension(thread, options)
 }

src/type-extensions/null.ts

@@ -0,0 +1,78 @@
+import { Decoration } from '../decoration'
+import { LuaReturn, LuaState } from '../types'
+import Global from '../global'
+import Thread from '../thread'
+import TypeExtension from '../type-extension'
+
+class NullTypeExtension extends TypeExtension<unknown> {
+    private gcPointer: number
+
+    public constructor(thread: Global) {
+        super(thread, 'js_null')
+
+        this.gcPointer = thread.lua.module.addFunction((functionStateAddress: LuaState) => {
+            // Throws a lua error which does a jump if it does not match.
+            const userDataPointer = thread.lua.luaL_checkudata(functionStateAddress, 1, this.name)
+            const referencePointer = thread.lua.module.getValue(userDataPointer, '*')
+            thread.lua.unref(referencePointer)
+
+            return LuaReturn.Ok
+        }, 'ii')
+
+        if (thread.lua.luaL_newmetatable(thread.address, this.name)) {
+            const metatableIndex = thread.lua.lua_gettop(thread.address)
+
+            // Mark it as uneditable
+            thread.lua.lua_pushstring(thread.address, 'protected metatable')
+            thread.lua.lua_setfield(thread.address, metatableIndex, '__metatable')
+
+            // Add the gc function
+            thread.lua.lua_pushcclosure(thread.address, this.gcPointer, 0)
+            thread.lua.lua_setfield(thread.address, metatableIndex, '__gc')
+
+            // Add an __index method that returns nothing.
+            thread.pushValue(() => null)
+            thread.lua.lua_setfield(thread.address, metatableIndex, '__index')
+
+            thread.pushValue(() => 'null')
+            thread.lua.lua_setfield(thread.address, metatableIndex, '__tostring')
+
+            thread.pushValue((self: unknown, other: unknown) => self === other)
+            thread.lua.lua_setfield(thread.address, metatableIndex, '__eq')
+        }
+        // Pop the metatable from the stack.
+        thread.lua.lua_pop(thread.address, 1)
+
+        // Create a new table, this is unique and will be the "null" value by attaching the
+        // metatable created above. The first argument is the target, the second options.
+        super.pushValue(thread, new Decoration<unknown>({}, {}))
+        // Put it into the global field named null.
+        thread.lua.lua_setglobal(thread.address, 'null')
+    }
+
+    public getValue(thread: Thread, index: number): null {
+        const refUserData = thread.lua.luaL_testudata(thread.address, index, this.name)
+        if (!refUserData) {
+            throw new Error(`data does not have the expected metatable: ${this.name}`)
+        }
+        return null
+    }
+
+    // any because LuaDecoration is not exported from the Lua lib.
+    public pushValue(thread: Thread, decoration: any): boolean {
+        if (decoration?.target !== null) {
+            return false
+        }
+        // Rather than pushing a new value, get the global "null" onto the stack.
+        thread.lua.lua_getglobal(thread.address, 'null')
+        return true
+    }
+
+    public close(): void {
+        this.thread.lua.module.removeFunction(this.gcPointer)
+    }
+}
+
+export default function createTypeExtension(thread: Global): TypeExtension<null> {
+    return new NullTypeExtension(thread)
+}