Unexpected memory leak report using arena allocator

[LewisGaul]

Excuse the length code snippet, but hopefully it's enlightening - I spent a long time narrowing down the problem from a real project!

In words: I'm getting an unexpected memory leak reported when using std.testing.allocator, seemingly coming from an allocation made by an arena allocator that I'm calling deinit() on.

Even more strange, is that this seems to be sensitive to:

• whether the function call that does the allocation is inside a struct declaration
• whether the arena allocator is referenced as a field in a struct when calling deinit()

Expected behaviour in the code below: no memory leaks.

Actual behaviour in the code below: memory leak for the first testcase only.

This is seen with fairly recent master branch, as well as on 0.7.1.

const std = @import("std");
const testing = std.testing;
const assert = std.debug.assert;

const Allocator = std.mem.Allocator;
const ArenaAllocator = std.heap.ArenaAllocator;
const StringArray = std.ArrayList([]const u8);

pub const ValueTree = struct {
    arena: ArenaAllocator,
    root: StringArray,
};

fn createArray(allocator: *Allocator, input: []const u8) StringArray {
    var array = StringArray.init(allocator);
    array.append(input) catch unreachable;
    return array;
}

test "memory leak" {
    var arena = ArenaAllocator.init(testing.allocator);
    var tree = ValueTree{
        .arena = arena,
        .root = createArray(&arena.allocator, "foo"),
    };
    tree.arena.deinit();
}

test "no memory leak - no function call in struct creation" {
    var arena = ArenaAllocator.init(testing.allocator);
    var in_array = createArray(&arena.allocator, "foo");
    var tree = ValueTree{
        .arena = arena,
        .root = in_array,
    };
    tree.arena.deinit();
}

test "no memory leak - direct arena deinit" {
    var arena = ArenaAllocator.init(testing.allocator);
    var tree = ValueTree{
        .arena = arena,
        .root = createArray(&arena.allocator, "foo"),
    };
    arena.deinit();
}

[yohannd1]

Here:

pub const ValueTree = struct {
    arena: ArenaAllocator,
    root: StringArray,
};

Shouldn't arena be a pointer to the allocator here, instead of the allocator itself?
I switched from ArenaAllocator to *ArenaAllocator (and switched the values passed to the trees from arena to &arena) and it seems to run fine, although I'm not sure if that was your intention or why the allocator is behaving differently because of that.

[fengb]

This is a slight footgun in how the arena currently works.

You copied the state of the arena before the allocation:

test "memory leak" {
    var arena = ArenaAllocator.init(testing.allocator);
    var tree = ValueTree{
        .arena = arena,                               // state copy
        .root = createArray(&arena.allocator, "foo"), // actual allocation
    };
    tree.arena.deinit();
}

This is fixed by either copying after all the allocations have happened or removing the copy altogether.

[LewisGaul]

Ahh of course, I see it now. I guess at this stage fine to just close the issue then, not sure how this might be made less of a gotcha in the future.

[SpexGuy]

The footgun here will be fixed when #7769 is implemented, which will make the copy a compile error.

[LewisGaul]

In case it's a useful reference to anyone coming across this issue in the future, here's how I stopped shooting myself in the foot thanks to the explanations above: LewisGaul/zig-nestedtext@df2a675