From 8b137404068aab391ef9b5e671bb519c83bc9a39 Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Fri, 24 Nov 2023 14:44:25 +0100 Subject: [PATCH] Free temporary slices This a breaking change. Before slices (strings) where kept in heap and the API functions didn't have to worry about that. Now those slices are freed after the function call, so the API functions needs to allocate (ie. copy) those slices if they want them to stay in memory. Signed-off-by: Francis Bouvier --- src/engines/v8/generate.zig | 32 ++++++++++++++++++++++++++- src/tests/proto_test.zig | 38 +++++++++++++++++++++++++-------- src/tests/types_native_test.zig | 12 +++++++---- 3 files changed, 68 insertions(+), 14 deletions(-) diff --git a/src/engines/v8/generate.zig b/src/engines/v8/generate.zig index 7ef1b7e..399a660 100644 --- a/src/engines/v8/generate.zig +++ b/src/engines/v8/generate.zig @@ -258,7 +258,6 @@ fn getArgs( // take all trailing JS arg as variadic members const rest_nb = js_args_nb - i + func.index_offset; const slice = alloc.alloc(arg_real.T, rest_nb) catch unreachable; - // TODO: alloc.free slice after func call var iter: usize = 0; while (iter < rest_nb) { const slice_value = getArg( @@ -285,6 +284,25 @@ fn getArgs( return args; } +fn freeArgs(comptime func: refl.Func, obj: anytype) !void { + inline for (func.args) |arg_T| { + + // free char slices + // the API functions will be responsible of copying the slice + // in their implementations if they want to keep it afterwards + if (arg_T.underT() == []u8 or arg_T.underT() == []const u8) { + utils.allocator.free(@field(obj, arg_T.name.?)); + } + + // free varidadic slices + if (try refl.Type.variadic(arg_T.underT()) != null) { + const val = @field(obj, arg_T.name.?).?; + // NOTE: variadic are optional by design + utils.allocator.free(@field(val, "slice")); + } + } +} + pub fn setNativeObject( alloc: std.mem.Allocator, comptime T_refl: refl.Struct, @@ -520,6 +538,9 @@ fn generateConstructor( isolate, ) catch unreachable; // TODO: we choose for now not throw user error + + // free memory if required + freeArgs(func, args) catch unreachable; } }.constructor; } @@ -585,6 +606,9 @@ fn generateGetter( ); }; info.getReturnValue().setValueHandle(js_val.handle); + + // free memory if required + freeArgs(func, args) catch unreachable; } }.getter; } @@ -658,6 +682,9 @@ fn generateSetter( // return to javascript the provided value info.getReturnValue().setValueHandle(raw_value.?); + + // free memory if required + freeArgs(func, args) catch unreachable; } }.setter; } @@ -728,6 +755,9 @@ fn generateMethod( }; info.getReturnValue().setValueHandle(js_val.handle); + // free memory if required + freeArgs(func, args) catch unreachable; + // sync callback // for test purpose, does not have any sense in real case if (comptime func.callback_index != null) { diff --git a/src/tests/proto_test.zig b/src/tests/proto_test.zig index dea9531..bc5b274 100644 --- a/src/tests/proto_test.zig +++ b/src/tests/proto_test.zig @@ -21,10 +21,16 @@ const Person = struct { pub const _AGE_MIN = 18; pub const _NATIONALITY = "French"; - pub fn constructor(_: std.mem.Allocator, first_name: []u8, last_name: []u8, age: u32) Person { + pub fn constructor(alloc: std.mem.Allocator, first_name: []u8, last_name: []u8, age: u32) Person { + + // alloc last_name slice to keep them after function returns + // NOTE: we do not alloc first_name on purpose to check freeArgs + const last_name_alloc = alloc.alloc(u8, last_name.len) catch unreachable; + @memcpy(last_name_alloc, last_name); + return .{ .first_name = first_name, - .last_name = last_name, + .last_name = last_name_alloc, .age = age, }; } @@ -55,6 +61,10 @@ const Person = struct { Person.allocTest(alloc) catch unreachable; } + pub fn get_nonAllocFirstName(self: Person) []const u8 { + return self.first_name; + } + pub fn set_age(self: *Person, age: u32) void { self.age = age; } @@ -99,8 +109,11 @@ const PersonPtr = struct { name: []u8, pub fn constructor(alloc: std.mem.Allocator, name: []u8) *PersonPtr { + const name_alloc = alloc.alloc(u8, name.len) catch unreachable; + @memcpy(name_alloc, name); + var person_ptr = alloc.create(PersonPtr) catch unreachable; - person_ptr.* = .{ .name = name }; + person_ptr.* = .{ .name = name_alloc }; return person_ptr; } @@ -108,8 +121,10 @@ const PersonPtr = struct { return self.name; } - pub fn set_name(self: *PersonPtr, name: []u8) void { - self.name = name; + pub fn set_name(self: *PersonPtr, alloc: std.mem.Allocator, name: []u8) void { + const name_alloc = alloc.alloc(u8, name.len) catch unreachable; + @memcpy(name_alloc, name); + self.name = name_alloc; } }; @@ -168,8 +183,10 @@ const PersonProtoCast = struct { return @ptrCast(child_ptr); } - pub fn constructor(first_name: []u8) PersonProtoCast { - return .{ .first_name = first_name }; + pub fn constructor(alloc: std.mem.Allocator, first_name: []u8) PersonProtoCast { + const first_name_alloc = alloc.alloc(u8, first_name.len) catch unreachable; + @memcpy(first_name_alloc, first_name); + return .{ .first_name = first_name_alloc }; } pub fn get_name(self: PersonProtoCast) []const u8 { @@ -182,8 +199,8 @@ const UserProtoCast = struct { pub const prototype = *PersonProtoCast; - pub fn constructor(first_name: []u8) UserProtoCast { - return .{ .not_proto = PersonProtoCast.constructor(first_name) }; + pub fn constructor(alloc: std.mem.Allocator, first_name: []u8) UserProtoCast { + return .{ .not_proto = PersonProtoCast.constructor(alloc, first_name) }; } }; @@ -249,6 +266,9 @@ pub fn exec( .{ .src = "p.allocator", .ex = "true" }, .{ .src = "p.UPPER", .ex = "true" }, .{ .src = "p.UPPERMETHOD()", .ex = "true" }, + // first name has not been allocated, so it's a normal behavior + // here we check that freeArgs works well + .{ .src = "p.nonAllocFirstName !== 'Francis'", .ex = "true" }, }; try tests.checkCases(js_env, &cases2); diff --git a/src/tests/types_native_test.zig b/src/tests/types_native_test.zig index 15349f1..a43b761 100644 --- a/src/tests/types_native_test.zig +++ b/src/tests/types_native_test.zig @@ -10,16 +10,20 @@ const tests = public.test_utils; const Brand = struct { name: []const u8, - pub fn constructor(name: []const u8) Brand { - return .{ .name = name }; + pub fn constructor(alloc: std.mem.Allocator, name: []const u8) Brand { + const name_alloc = alloc.alloc(u8, name.len) catch unreachable; + @memcpy(name_alloc, name); + return .{ .name = name_alloc }; } pub fn get_name(self: Brand) []const u8 { return self.name; } - pub fn set_name(self: *Brand, name: []u8) void { - self.name = name; + pub fn set_name(self: *Brand, alloc: std.mem.Allocator, name: []u8) void { + const name_alloc = alloc.alloc(u8, name.len) catch unreachable; + @memcpy(name_alloc, name); + self.name = name_alloc; } };