Skip to content

Commit

Permalink
Free temporary slices
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
francisbouvier committed Nov 24, 2023
1 parent ed78daa commit 8b13740
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 14 deletions.
32 changes: 31 additions & 1 deletion src/engines/v8/generate.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -585,6 +606,9 @@ fn generateGetter(
);
};
info.getReturnValue().setValueHandle(js_val.handle);

// free memory if required
freeArgs(func, args) catch unreachable;
}
}.getter;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down
38 changes: 29 additions & 9 deletions src/tests/proto_test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -99,17 +109,22 @@ 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;
}

pub fn get_name(self: PersonPtr) []u8 {
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;
}
};

Expand Down Expand Up @@ -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 {
Expand All @@ -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) };
}
};

Expand Down Expand Up @@ -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);

Expand Down
12 changes: 8 additions & 4 deletions src/tests/types_native_test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};

Expand Down

0 comments on commit 8b13740

Please sign in to comment.