CODEWITHSUNDEEP
memory-leakszig
jackdbd
jackdbd

Reputation: 5043

How to avoid memory leaks when building a slice of slices using ArrayLists

I'm trying to build a slice of slices using multiple std.ArrayLists.

The code below works, but the memory allocator std.testing.allocator warns me of a memory leaks wherever I append new elements to a sublist.

const std = @import("std");
const mem = std.mem;

fn sliceOfSlices(allocator: *mem.Allocator) ![][]usize {
    var list = std.ArrayList([]usize).init(allocator);

    var i: usize = 0;
    while (i < 3) : (i += 1) {
        var sublist = std.ArrayList(usize).init(allocator);
        // errdefer sublist.deinit(); // here?
        var n: usize = 0;
        while (n < 5) : (n += 1) {
            try sublist.append(n); // leaks
            // errdefer sublist.deinit(); // here?
            // errdefer allocator.free(sublist.items);
        }
        try list.append(sublist.toOwnedSlice());
    }
    return list.toOwnedSlice();
}

const testing = std.testing;

test "memory leaks" {
    const slice = try sliceOfSlices(testing.allocator);
    testing.expectEqual(@intCast(usize, 3), slice.len);
    testing.expectEqual(@intCast(usize, 5), slice[0].len);
}

I tried to use errdefer in several places to free the allocated sublist, but it didn't work. From the documentation it seems a lifetime issue, but I'm not sure how to handle it.

the std.ArrayList(T).items slice has a lifetime that remains valid until the next time the list is resized, such as by appending new elements. — https://ziglang.org/documentation/master/#Lifetime-and-Ownership

What's the appropriate error handling when list.append() fails?

Upvotes: 3

Views: 1859

Answers (1)

chi
chi

Reputation: 309

I am a beginner to zig so perhaps I am totally wrong here, but I think the reason why you are getting the memory leak is not because something failed!

As you use ArrayList its memory is allocated via an allocator, the memory has explicitly to be freed at end of usage. For an ArrayList you could simply use the deinit() function. However as your function sliceOfSlices() converts that ArrayList wrapper to a slice, you have to use testing.allocator.free(slice) to get rid of the memory used by that slice.

But note: every element of your slice is itself a slice (or a pointer to it). Also obtained via ArrayList.toOwnedSlice(). Therefore those slices you have also to get rid of, before you can deallocate the containing slice.

So I would change your test to

test "memory leaks" {
    const slice = try sliceOfSlices(testing.allocator);
    defer {
        for (slice) |v| {
            testing.allocator.free(v);
        }
        testing.allocator.free(slice);
    }
    testing.expectEqual(@intCast(usize, 3), slice.len);
    testing.expectEqual(@intCast(usize, 5), slice[0].len);
}

and now no memory leak should occur anymore.

Perhaps somebody knows a better solution, but lacking experience here, this would be the way to go, IMO.

And after some thinking, answering your question what to do in case of an error, I would rewrite your function sliceOfSlices() to

fn sliceOfSlices(allocator: *mem.Allocator) ![][]usize {
    var list = std.ArrayList([]usize).init(allocator);
    errdefer {
        for (list.items) |slice| {
            allocator.free(slice);
        }
        list.deinit();
    }

    var i: usize = 0;
    while (i < 3) : (i += 1) {
        var sublist = std.ArrayList(usize).init(allocator);
        errdefer sublist.deinit();

        var n: usize = 0;
        while (n < 5) : (n += 1) {
            try sublist.append(n);
        }

        try list.append(sublist.toOwnedSlice());
    }

    return list.toOwnedSlice();
}

Now if any error happened in your function, both list and sublist should be cleaned up properly. Still if no error was returned from the function, your calling code would be responsible for the cleanup to avoid memory leaks like implemented in the test block above.

Upvotes: 3

Related Questions