Parsing memory fixes (#63)

* updated memory management system to use toAny and toAnyAlloc

* performance optimization

* small change to see if tests pass now

* fixed bug

* added stress test

* added stress test and fixed bugs causing lua stack overflows

* bug fix

* removed std.debug.panic

Author: VisenDev

src/lib.zig

@@ -571,6 +571,21 @@ pub const CWarnFn = switch (lang) {
 /// The type of the writer function used by `Lua.dump()`
 pub const CWriterFn = *const fn (state: ?*LuaState, buf: ?*const anyopaque, size: usize, data: ?*anyopaque) callconv(.C) c_int;
 
+/// For bundling a parsed value with an arena allocator
+/// Copied from std.json.Parsed
+pub fn Parsed(comptime T: type) type {
+    return struct {
+        arena: *std.heap.ArenaAllocator,
+        value: T,
+
+        pub fn deinit(self: @This()) void {
+            const allocator = self.arena.child_allocator;
+            self.arena.deinit();
+            allocator.destroy(self.arena);
+        }
+    };
+}
+
 /// A Zig wrapper around the Lua C API
 /// Represents a Lua state or thread and contains the entire state of the Lua interpreter
 pub const Lua = struct {
@@ -1658,8 +1673,7 @@ pub const Lua = struct {
         }
     }
 
-    /// Similar to `Lua.setTable()` but does a raw asskdjfal;sdkfjals;dkfj;dk:q
-    /// gnment (without metamethods)
+    /// Similar to `Lua.setTable()` but does a raw assignment (without metamethods)
     /// See https://www.lua.org/manual/5.4/manual.html#lua_rawset
     pub fn rawSetTable(lua: *Lua, index: i32) void {
         c.lua_rawset(lua.state, index);
@@ -3145,7 +3159,42 @@ pub const Lua = struct {
 
     /// Converts the specified index of the lua stack to the specified
     /// type if possible and returns it
-    pub fn toAny(lua: *Lua, comptime T: type, index: i32) !T {
+    /// Allocates memory if necessary
+    pub fn toAnyAlloc(lua: *Lua, comptime T: type, index: i32) !Parsed(T) {
+        var parsed = Parsed(T){
+            .arena = try lua.allocator().create(std.heap.ArenaAllocator),
+            .value = undefined,
+        };
+        errdefer lua.allocator().destroy(parsed.arena);
+        parsed.arena.* = std.heap.ArenaAllocator.init(lua.allocator());
+        errdefer parsed.arena.deinit();
+
+        parsed.value = try lua.toAnyInternal(T, parsed.arena.allocator(), true, index);
+
+        return parsed;
+    }
+
+    /// Converts the specified index of the lua stack to the specified
+    /// type if possible and returns it
+    /// Does not allocate any memory, if memory allocation is needed (such as for parsing slices)
+    /// use toAnyAlloc
+    pub inline fn toAny(lua: *Lua, comptime T: type, index: i32) !T {
+        return lua.toAnyInternal(T, null, false, index);
+    }
+
+    /// Converts the specified index of the lua stack to the specified
+    /// type if possible and returns it
+    /// optional allocator
+    fn toAnyInternal(lua: *Lua, comptime T: type, a: ?std.mem.Allocator, comptime allow_alloc: bool, index: i32) !T {
+        const stack_size_on_entry = lua.getTop();
+        defer {
+            if (lua.getTop() != stack_size_on_entry) {
+                std.debug.print("Type that filed to parse was: {any}\n", .{T});
+                std.debug.print("Expected stack size: {}, Actual Stack Size: {}\n\n", .{ stack_size_on_entry, lua.getTop() });
+                @panic("internal parsing error");
+            }
+        }
+
         switch (@typeInfo(T)) {
             .Int => {
                 switch (comptime lang) {
@@ -3183,7 +3232,10 @@ pub const Lua = struct {
                     }
                 } else switch (info.size) {
                     .Slice, .Many => {
-                        return try lua.toSlice(info.child, index);
+                        if (!allow_alloc) {
+                            @compileError("toAny cannot allocate memory, try using toAnyAlloc");
+                        }
+                        return try lua.toSlice(info.child, a.?, index);
                     },
                     else => {
                         return try lua.toUserdata(info.child, index);
@@ -3194,7 +3246,7 @@ pub const Lua = struct {
                 return lua.toBoolean(index);
             },
             .Enum => |info| {
-                const string = try lua.toAny([]const u8, index);
+                const string = try lua.toAnyInternal([]const u8, a, allow_alloc, index);
                 inline for (info.fields) |enum_member| {
                     if (std.mem.eql(u8, string, enum_member.name)) {
                         return @field(T, enum_member.name);
@@ -3203,14 +3255,13 @@ pub const Lua = struct {
                 return error.InvalidEnumTagName;
             },
             .Struct => {
-                return try lua.toStruct(T, index);
+                return try lua.toStruct(T, a, allow_alloc, index);
             },
             .Optional => {
                 if (lua.isNil(index)) {
-                    lua.pop(1);
                     return null;
                 } else {
-                    return try lua.toAny(@typeInfo(T).Optional.child, index);
+                    return try lua.toAnyInternal(@typeInfo(T).Optional.child, a, allow_alloc, index);
                 }
             },
             else => {
@@ -3220,27 +3271,31 @@ pub const Lua = struct {
     }
 
     /// Converts a lua array to a zig slice, memory is owned by the caller
-    fn toSlice(lua: *Lua, comptime ChildType: type, raw_index: i32) ![]ChildType {
+    fn toSlice(lua: *Lua, comptime ChildType: type, a: std.mem.Allocator, raw_index: i32) ![]ChildType {
         const index = lua.absIndex(raw_index);
 
         if (!lua.isTable(index)) {
             return error.ValueNotATable;
         }
 
         const size = lua.rawLen(index);
-        var result = try lua.allocator().alloc(ChildType, size);
+        var result = try a.alloc(ChildType, size);
 
         for (1..size + 1) |i| {
             _ = try lua.pushAny(i);
             _ = lua.getTable(index);
-            result[i - 1] = try lua.toAny(ChildType, -1);
+            result[i - 1] = try lua.toAnyInternal(ChildType, a, true, -1);
+            lua.pop(1);
         }
 
         return result;
     }
 
     /// Converts value at given index to a zig struct if possible
-    fn toStruct(lua: *Lua, comptime T: type, raw_index: i32) !T {
+    fn toStruct(lua: *Lua, comptime T: type, a: ?std.mem.Allocator, comptime allow_alloc: bool, raw_index: i32) !T {
+        const stack_size_on_entry = lua.getTop();
+        defer std.debug.assert(lua.getTop() == stack_size_on_entry);
+
         const index = lua.absIndex(raw_index);
 
         if (!lua.isTable(index)) {
@@ -3249,27 +3304,32 @@ pub const Lua = struct {
         std.debug.assert(lua.typeOf(index) == .table);
 
         var result: T = undefined;
+
         inline for (@typeInfo(T).Struct.fields) |field| {
             const field_name = comptime field.name ++ "";
             _ = lua.pushString(field_name);
-            std.debug.assert(lua.typeOf(index) == .table);
+
             const lua_field_type = lua.getTable(index);
             if (lua_field_type == .nil) {
                 if (field.default_value) |default_value| {
                     @field(result, field.name) = @as(*const field.type, @ptrCast(@alignCast(default_value))).*;
                 } else {
+                    lua.pop(1);
                     return error.LuaTableMissingValue;
                 }
             } else {
-                @field(result, field.name) = try lua.toAny(field.type, -1);
+                const stack_size_before_call = lua.getTop();
+                @field(result, field.name) = try lua.toAnyInternal(field.type, a, allow_alloc, -1);
+                std.debug.assert(stack_size_before_call == lua.getTop());
             }
+            lua.pop(1); //pop the value off the stack
         }
 
         return result;
     }
 
-    ///automatically calls a lua function with the given arguments
-    pub fn autoCall(lua: *Lua, comptime ReturnType: type, func_name: [:0]const u8, args: anytype) !ReturnType {
+    /// Calls a function and pushes its return value to the top of the stack
+    fn autoCallAndPush(lua: *Lua, comptime ReturnType: type, func_name: [:0]const u8, args: anytype) !void {
         if (try lua.getGlobal(func_name) != LuaType.function) return error.InvalidFunctionName;
 
         inline for (args) |arg| {
@@ -3278,9 +3338,22 @@ pub const Lua = struct {
 
         const num_results = if (ReturnType == void) 0 else 1;
         try lua.protectedCall(args.len, num_results, 0);
-        defer lua.setTop(0);
+    }
 
-        return lua.toAny(ReturnType, -1);
+    ///automatically calls a lua function with the given arguments
+    pub fn autoCall(lua: *Lua, comptime ReturnType: type, func_name: [:0]const u8, args: anytype) !ReturnType {
+        try lua.autoCallAndPush(ReturnType, func_name, args);
+        const result = try lua.toAny(ReturnType, -1);
+        lua.setTop(0);
+        return result;
+    }
+
+    ///automatically calls a lua function with the given arguments
+    pub fn autoCallAlloc(lua: *Lua, comptime ReturnType: type, func_name: [:0]const u8, args: anytype) !Parsed(ReturnType) {
+        try lua.autoCallAndPush(ReturnType, func_name, args);
+        const result = try lua.toAnyAlloc(ReturnType, -1);
+        lua.setTop(0);
+        return result;
     }
 
     //automatically generates a wrapper function
@@ -3296,9 +3369,23 @@ pub const Lua = struct {
                 var parameters: std.meta.ArgsTuple(@TypeOf(function)) = undefined;
 
                 inline for (info.Fn.params, 0..) |param, i| {
-                    parameters[i] = lua.toAny(param.type.?, (i + 1)) catch |err| {
-                        lua.raiseErrorStr(@errorName(err), .{});
-                    };
+                    const param_info = @typeInfo(param.type.?);
+                    //only use the overhead of creating the arena allocator if needed
+                    if (comptime param_info == .Pointer and param_info.Pointer.size != .One) {
+                        const parsed = lua.toAnyAlloc(param.type.?, (i + 1)) catch |err| {
+                            lua.raiseErrorStr(@errorName(err), .{});
+                        };
+
+                        defer parsed.deinit();
+
+                        parameters[i] = parsed.value;
+                    } else {
+                        const parsed = lua.toAny(param.type.?, (i + 1)) catch |err| {
+                            lua.raiseErrorStr(@errorName(err), .{});
+                        };
+
+                        parameters[i] = parsed;
+                    }
                 }
 
                 if (@typeInfo(info.Fn.return_type.?) == .ErrorUnion) {
@@ -3332,6 +3419,13 @@ pub const Lua = struct {
         return try lua.toAny(ReturnType, -1);
     }
 
+    /// get any lua global
+    /// can allocate memory
+    pub fn getAlloc(lua: *Lua, comptime ReturnType: type, name: [:0]const u8) !Parsed(ReturnType) {
+        _ = try lua.getGlobal(name);
+        return try lua.toAnyAlloc(ReturnType, -1);
+    }
+
     ///set any lua global
     pub fn set(lua: *Lua, name: [:0]const u8, value: anytype) !void {
         try lua.pushAny(value);

src/tests.zig

@@ -2510,11 +2510,11 @@ test "toAny slice" {
     ;
     try lua.doString(program);
     _ = try lua.getGlobal("list");
-    const sliced = try lua.toAny([]u32, -1);
-    defer lua.allocator().free(sliced);
+    const sliced = try lua.toAnyAlloc([]u32, -1);
+    defer sliced.deinit();
 
     try testing.expect(
-        std.mem.eql(u32, &[_]u32{ 1, 2, 3, 4, 5 }, sliced),
+        std.mem.eql(u32, &[_]u32{ 1, 2, 3, 4, 5 }, sliced.value),
     );
 }
 
@@ -2637,8 +2637,58 @@ test "autoCall" {
     ;
 
     try lua.doString(program);
-    const sum = try lua.autoCall(usize, "add", .{ 1, 2 });
-    try std.testing.expect(3 == sum);
+
+    for (0..100) |_| {
+        const sum = try lua.autoCall(usize, "add", .{ 1, 2 });
+        try std.testing.expect(3 == sum);
+    }
+
+    for (0..100) |_| {
+        const sum = try lua.autoCallAlloc(usize, "add", .{ 1, 2 });
+        defer sum.deinit();
+        try std.testing.expect(3 == sum.value);
+    }
+}
+
+test "autoCall stress test" {
+    var lua = try Lua.init(&testing.allocator);
+    defer lua.deinit();
+
+    const program =
+        \\function add(a, b)
+        \\   return a + b
+        \\end
+        \\
+        \\
+        \\function KeyBindings()
+        \\
+        \\   local bindings = {
+        \\      {['name'] = 'player_right', ['key'] = 'a'},
+        \\      {['name'] = 'player_left',  ['key'] = 'd'},
+        \\      {['name'] = 'player_up',    ['key'] = 'w'},
+        \\      {['name'] = 'player_down',  ['key'] = 's'},
+        \\      {['name'] = 'zoom_in',      ['key'] = '='},
+        \\      {['name'] = 'zoom_out',     ['key'] = '-'},
+        \\      {['name'] = 'debug_mode',   ['key'] = '/'},
+        \\   }
+        \\
+        \\   return bindings
+        \\end
+    ;
+
+    try lua.doString(program);
+
+    const ConfigType = struct {
+        name: []const u8,
+        key: []const u8,
+        shift: bool = false,
+        control: bool = false,
+    };
+
+    for (0..100) |_| {
+        const sum = try lua.autoCallAlloc([]ConfigType, "KeyBindings", .{});
+        defer sum.deinit();
+    }
 }
 
 test "get set" {
@@ -2667,6 +2717,8 @@ test "array of strings" {
 
     try lua.doString(program);
 
-    const strings = try lua.autoCall([]const []const u8, "strings", .{});
-    lua.allocator().free(strings);
+    for (0..100) |_| {
+        const strings = try lua.autoCallAlloc([]const []const u8, "strings", .{});
+        defer strings.deinit();
+    }
 }