Documentation says if arena is passed as an allocator, it’s better to use parseFromSliceLeaky(). I do pass arena, but I don’t have to in this function:
I saw that parseFromSlice uses @memcpy() but I wasn’t able to follow it fully. Does adding that defer parsed.deinit() make use-after-free here? I am calling allocator free(reader.buf) to free a buf passed in a return of a struct
parseFromSlice makes an arena arround the allocator you give it, then just calls parseFromSliceLeaky.
parseFromSlice returns a Parsed(T) which contains the arena it created, this is so you can properly free all the memory, which is what parsed.deinit() does.
So yes, calling parsed.deinit before returning the value may cause use after frees if the parsed value contains pointers.
The api allows you to control when allocation happens via ParseOptions.allocWhen, the default behaviour depends on the api you use; for parseFromSlice it is .alloc_if_needed which means the parsed value may point into the source buffer, so freeing the buffer may cause use after frees.
Thanks for the reply.
I changed my allocator to gpa, but I was not able to trigger the crash. I did however crashed it when I set allocate = .alloc_always with defer parsed.deinit().
I understand why it’s not crashing with .alloc_if_needed, it operates with slices on a data that is being preserved in a buf and passed into main, where I’m freeing it after it’s used.
I do not understand why the actual slices are not being destroyed with deinit() and why it is crashing when I force it to allocate?
A pointer is just an address to memory, it is completely divorced from the allocators’ management of memory; freeing memory does not get rid of the address, it just informs the allocator that you are no longer using that address, which is why it is erroneous to continue to use it after.
It is crashing because the allocator chose to give the memory back to the OS, so the address is no longer valid, the allocator could also have chosen to keep it to reuse, then the address would still be valid and wouldn’t crash, though could cause unintended behaviour (e.g. memory corruption) if other code gets that memory.
Alright, so when .alloc_awlays is set, it asks for a new memory that deinit() gives back to the OS. That’s valid.
In case of .alloc_if_needed it works with addresses that are pointing to a memory of my buf which is not owned by a wrapper and it’s managed externally so the data is safe. Now, deinit() tells that addresses are no longer used but it does not do anything with them. What is managing addresses?
You, it’s your responsibility to be aware of when memory is, and isn’t, valid.
This is what manual memory management means.
That does not mean you should be hyper aware, that just isn’t possible, rather you should design your code around to minimise what you need to be aware of.
When dealing string, or any other things in heap, such as array, HashMap, etc … , I like to think in a way that “Do they have a manager or not?”. It’s always recommended that the defer deinit should be in the next line of the creation of the manager. And the manager should live as long as the string or whatever in heap.
parseFromSlice returns a struct Parsed, which already has an ArenaAllocator to hold the string for you, so the string is managed. The Parsed struct is a manager. The value will live as long as its manager – the Parse. (This is what they called lifetime management).
So in this case, the Parsed should be directly used in the rest of our codes. For example, it can be used in the main loop, or in another complex struct, or in another complex function, or anything the codes that will use the value for a longer time, not just another pure function to return the value.
If we want to create our own library to return anything that’s in heap, we can either create our own struct, which includes an ArenaAllocator, to be a manager of the string, then we use the parseFromSliceLeaky inside. Or we can create a pure function like what you just did, but use the parseFromSliceLeaky, and let the users to decide how to have a “manager” for them. Either way works.
In Zig’s std, there are some similar things, they called them BlaBlaBlaWhateverManaged, or something like that. They are using the same idea.
Alright, Thank you all for replies and indeed, I was creating a use-after-free but I saw using deprecated DebugAllocator which was not able to catch it.
Switching to SafeAllocator did however. It was crashing with both .alloc_if_needed and .alloc_always.
This is what I came up with:
const Reader = struct {
parsed: std.json.Parsed(Config),
buf: []u8,
/// Helper to get quick access to the config fields
pub fn config(self: *const Reader) *const Config {
return &self.parsed.value;
}
/// Deinit of the parsed structure and a buffer that holds the data
pub fn deinit(self: Reader, gpa: std.mem.Allocator) void {
self.parsed.deinit();
gpa.free(self.buf);
}
};
/// Read and parse the config file
pub fn readFile(io: std.Io, gpa: std.mem.Allocator, path: []const u8) !Reader {
const file = try std.Io.Dir.cwd().openFile(io, path, .{
.mode = .read_only,
.lock = .exclusive,
});
defer file.close(io);
const buf = try gpa.alloc(u8, try file.length(io));
// Only on error, it holds actual data where parser is pointing to
errdefer gpa.free(buf);
var reader = file.reader(io, buf);
// Read all content of a file into buffer
try reader.interface.readSliceAll(buf);
const parsed = try std.json.parseFromSlice(
Config,
gpa,
buf,
.{ .ignore_unknown_fields = true, .allocate = .alloc_if_needed },
);
return .{ .parsed = parsed, .buf = buf };
}
That uses best of both. I’m deiniting in main like I wanted and freeing a buf, and I’m using already existing Zig’s own Parsed struct that is tracking all the other allocations that may occur.
Ya this one works perfectly because the Reader struct now is the holder of the Parsed struct, and the Parsed struct is the holder of your value. The Reader acts like a higher manager of the Parsed and the buf. So when you use the Reader in your main, all of them alive at the same time: Reader, Parsed and its value, and the buf.