ArrayListUnmanaged memory leak

Help
1

The append method says that it invalidates item pointers if more memory is needed, is this why im having a memory leak?

pub fn runDiagnosis(allocator: std.mem.Allocator, progress_node: std.Progress.Node, diagnosis: *Diagnosis) void {
    var tok = std.mem.tokenizeScalar(u8, formulaes, '\n');

    var buf: [1024]u8 = undefined;
    while (tok.next()) |formulae| {
        const node_name = std.fmt.bufPrint(&buf, "Installing: {s} with Homebrew", .{formulae}) catch "";

        const run_result = std.process.Child.run(.{
            .progress_node = progress_node.start(node_name, 0),
            .allocator = allocator,
            .argv = &.{ "which", formulae },
        }) catch return;

        defer allocator.free(run_result.stdout); // 󱔾  Free stdout
        defer allocator.free(run_result.stderr); // 󱔾  Free stderr

        // Check if the command was successfull
        const exit_code = run_result.term.Exited;

        // BUG: Leak
        if (exit_code != 0) {
            diagnosis.missing_software.append(allocator, formulae) catch continue;
        }
    }
}

Im calling deinit on main:

    // Diagnosis Struct --------------------------------------------------------
    var diag: Diagnosis = undefined;
    diag.init();
    defer diag.deinit(allocator);

    try pool.spawn(homebrew.runDiagnosis, .{ allocator, root_node, &diag });
    wg.wait();

    for (diag.missing_software.items) |missing_formulae| {
        std.debug.print("Missing: {s}", .{missing_formulae});
    }

That empties the array and frees the memory:

pub fn deinit(self: *Self, allocator: std.mem.Allocator) void {
    self.missing_software.clearAndFree(allocator);
}
2

Do you want us to guess what Diagnosis is? :wink:

3

Use the DebugAllocator and show us / look at the leak error message.
That should show you where the leak was allocated and thus where you are missing a free.

1 Like
4

Sorry, I forgot to include the diagnosis struct:

// diagnosis.zig

const std = @import("std");
const Self = @This();

missing_formulae: std.ArrayList([]const u8),

pub fn init(self: *Self) void {
    self.* = .{
        .missing_formulae = .empty,
    };
}

pub fn deinit(self: *Self, allocator: std.mem.Allocator) void {
    self.missing_formulae.clearAndFree(allocator);
}

Here’s the error message:

error(gpa): memory address 0x7b23ae760180 leaked:
/home/linuxbrew/.linuxbrew/Cellar/zig/0.15.1/lib/zig/std/array_list.zig:1231:56: 0x119830e in ensureTotalCapacityPrecise (std.zig)
                const new_memory = try gpa.alignedAlloc(T, alignment, new_capacity);
                                                       ^
/home/linuxbrew/.linuxbrew/Cellar/zig/0.15.1/lib/zig/std/array_list.zig:1207:51: 0x1189275 in ensureTotalCapacity (std.zig)
            return self.ensureTotalCapacityPrecise(gpa, growCapacity(self.capacity, new_capacity));
                                                  ^
/home/linuxbrew/.linuxbrew/Cellar/zig/0.15.1/lib/zig/std/array_list.zig:1261:41: 0x117ad2b in addOne (std.zig)
            try self.ensureTotalCapacity(gpa, newlen);
                                        ^
/home/linuxbrew/.linuxbrew/Cellar/zig/0.15.1/lib/zig/std/array_list.zig:894:49: 0x11724b6 in append (std.zig)
            const new_item_ptr = try self.addOne(gpa);
                                                ^
/home/kacaii/ghq/github.com/Kacaii/vm_diagnosis/src/homebrew/homebrew.zig:28:46: 0x11655d9 in runDiagnosis (homebrew.zig)
            diagnosis.missing_formulae.append(allocator, formulae) catch continue;
                                             ^
/home/linuxbrew/.linuxbrew/Cellar/zig/0.15.1/lib/zig/std/Thread/Pool.zig:230:39: 0x115aa12 in runFn (std.zig)
            @call(.auto, func, closure.arguments);
                                      ^
5

Could it be that the lifetime of your allocator is shorter than the lifetime of your actual code? (For example if your syncronization/work-group isn’t configured correctly?)

If you run debug_alloc.deinit() before the other things were actually deinit-ed than it would complain about leaks too.

That is the only thing I can currently think of, you also could add a print or panic to this deinit to make sure that it actually gets called, before the leak detection happens:

pub fn deinit(self: *Self, allocator: std.mem.Allocator) void {
    self.missing_formulae.clearAndFree(allocator);
}

Using a debugger to look at it would make sense too.

1 Like
6

Thanks for your attention! Im running everything in a threadpool, but idk if that influences the way memory is handled.

The main goal is to find what softwares / configurations are missing on a machine and report them at the end, like missing ssh config, things like that.

Here’s the main file, sorry for not including it before.

// main.zig

pub fn main() !void {

    //   Nodes-----------------------------------------------------------------
    const root_node = std.Progress.start(.{ .root_name = "  Running Healthchecks" });
    defer root_node.end();

    //   Allocator ------------------------------------------------------------
    var gpa: std.heap.DebugAllocator(.{}) = .init;
    const allocator, const is_debug = switch (builtin.mode) {
        .Debug, .ReleaseSafe => .{ gpa.allocator(), true },
        .ReleaseFast, .ReleaseSmall => .{ std.heap.smp_allocator, false },
    };

    defer if (is_debug) {
        _ = gpa.deinit();
    };

    //   Multithreading -------------------------------------------------------
    var wg: std.Thread.WaitGroup = .{};

    var pool: std.Thread.Pool = undefined;
    try pool.init(.{
        .allocator = allocator,
        .n_jobs = std.Thread.getCpuCount() catch 1,
    });
    defer pool.deinit();

    // Diagnosis Struct --------------------------------------------------------
    var diag: Diagnosis = undefined;
    diag.init();
    defer diag.deinit(allocator);

    try pool.spawn(homebrew.runDiagnosis, .{ allocator, root_node, &diag });
    wg.wait();

    for (diag.missing_formulae.items) |formulae| {
        std.debug.print("Missing: {s}", .{formulae});
    }
}
7

I don’t see you using the wait group anywhere, for example by calling wg.start() or wg.startMany(), which would mean that the wait group becomes a noop (doing nothing), because it doesn’t have anything to wait on and that would mean that the main thread doesn’t wait for the sub-threads to complete, thus calling gpa.deinit() too early.

I think you need to use pool.spawnWg instead of pool.spawn, here is an example: Zig Cookbook

try pool.spawnWg(&wg, homebrew.runDiagnosis, .{ allocator, root_node, &diag });
2 Likes
8

It worked!!
I was looking at the wrong direction this whole time, thank you so much <3

1 Like