Crash and deadlock when using ArenaAllocator with GeneralPurposeAllocator

Help
, ,
1

Hi,

I’ve been experimenting with ArenaAllocator backed by GeneralPurposeAllocator and ran into confusing crashes. Here’s a minimal example:

const std = @import("std");

const Content = struct {
    gpa: std.heap.GeneralPurposeAllocator(.{}),
    arena: std.heap.ArenaAllocator,
    inner: std.ArrayList(u8),

    const Self = @This();

    fn new() !Self {
        var gpa = std.heap.GeneralPurposeAllocator(.{}){};
        var arena = std.heap.ArenaAllocator.init(gpa.allocator());
        const allocator = arena.allocator();

        var inner = try std.ArrayList(u8).initCapacity(allocator, 10);
        try inner.append(allocator, 1);

        return Self{
            .gpa = gpa,
            .arena = arena,
            .inner = inner,
        };
    }

    fn deinit(self: *Self) void {
        self.arena.deinit();
    }
};

test "content" {
    var content = try Content.new();
    content.deinit();
}

When I run this test, I get a crash:

thread panic: reached unreachable code
/usr/local/Cellar/zig/0.15.2/lib/zig/std/debug.zig:559:14: in assert
    if (!ok) unreachable; // assertion failure
/usr/local/Cellar/zig/0.15.2/lib/zig/std/heap/debug_allocator.zig:951:27: in free
    assert(self.buckets[size_class_index] == bucket);
...
/Users/.../allocator_learn.zig:27:26: in deinit
        self.arena.deinit();

Occasionally, instead of the above, I also see a panic with the message:

thread #1, stop reason = signal SIGABRT
frame #4: debug.defaultPanic(msg="Deadlock detected")
frame #5: Thread.Mutex.DebugImpl.lock

So sometimes it’s an assertion failure inside debug_allocator.zig, other times it’s a deadlock panic.

Interestingly, if I don’t store gpa inside the struct (only keep arena), the program runs fine:

zig

const Content = struct {
    arena: std.heap.ArenaAllocator,
    inner: std.ArrayList(u8),
    ...
};

This version works without crashing.

  • Why does storing gpa inside the struct cause arena.deinit() to crash or deadlock, while leaving gpa as a local variable in new() works fine?

  • I thought the correct order was simply arena.deinit() then gpa.deinit(), but here I’m not even calling gpa.deinit(). Is there some subtle interaction between ArenaAllocator and GeneralPurposeAllocator’s debug tracking that makes this unsafe?

  • What’s the recommended ownership/lifetime pattern for using ArenaAllocator backed by GPA?

Any insights would be greatly appreciated. Thanks!

2

Interestingly it works reliably for me, so I don’t quite know what’s wrong for you. But there are still a few things at play here. TLDR: pointer to stack memory

When you call gpa.allocator() it internally does just this:

        pub fn allocator(self: *Self) Allocator {
            return .{
                .ptr = self,
                .vtable = &.{
                    .alloc = alloc,
                    .resize = resize,
                    .remap = remap,
                    .free = free,
                },
            };
        }

Meaning it takes a pointer and saves that in the interface. When you then call it to ArenaAllocator.init this internally also saves this pointer. But your gpa is just a stack variable that is (conceptually) destroyed after the new function returns. So you have a dangling pointer and anything can happen after that.

That’s why you see in zig code in the wild that these foundational allocators like a “global” gpa are always created in main and just passed down to where they’re needed.

I would do it like this (matching your code):

const std = @import("std");

const Content = struct {
    arena: std.heap.ArenaAllocator,
    inner: std.ArrayList(u8),

    const Self = @This();

    fn init(gpa: std.mem.Allocator) !Self {
        var arena = std.heap.ArenaAllocator.init(gpa);

        var inner = try std.ArrayListUnmanaged(u8).initCapacity(arena.allocator(), 10);
        inner.appendAssumeCapacity(1);

        return Self{
            .arena = arena,
            .inner = inner,
        };
    }

    fn deinit(self: *Self) void {
        self.arena.deinit();
    }
};

test "content" {
    var gpa_inst = std.heap.GeneralPurposeAllocator(.{}){};
    defer _ = gpa_inst.deinit();
    const gpa = gpa_inst.allocator();

    var content = try Content.init(gpa);
    defer content.deinit();
}

Also note how it’s okay that arena is created inside the initialization because we don’t store a pointer to it just the value itself.

PS: welcome to the forum

5 Likes
3 
const std = @import("std");

const Content = struct {
    gpa: std.heap.GeneralPurposeAllocator(.{}),
    arena: std.heap.ArenaAllocator,
    inner: std.ArrayList(u8),

    const Self = @This();

    fn init(self: *Self) !void{
        self.gpa = std.heap.GeneralPurposeAllocator(.{}){};
        self.arena = std.heap.ArenaAllocator.init(self.gpa.allocator());
        const allocator = self.arena.allocator();
        self.inner = try std.ArrayList(u8).initCapacity(allocator, 10);
        try self.inner.append(allocator, 1);
        try inner.append(allocator, 1);
    }

    fn deinit(self: *Self) void {
        self.arena.deinit();
    }
};

test "content" {
    var content: Content = undefined;
    try content.init();
    defer content.deinit();
}

Here is another example of initializing a ‘self-referential’ struct. Such a struct can only be initialized in the form above because gpa.allocator() implicitly contains a pointer to gpa. An arena created using gpa.allocator() on the stack internally stores a pointer to gpa on the stack, which changes when gpa is copied.

OP’s example would likely cause a compilation error in version 0.16.

1 Like
4

Thank you very much.

The gpa variable does indeed get released prematurely, causing subsequent pointers to the gpa variable to be abnormal. What I’m curious about is why, after I assign a value to gpa (which constructs a new stack variable, so the old gpa variable will still be released) and save it in Content, the exception can be consistently triggered, but the work is normal when this gpa variable is not saved.

5

What I can understand now is that the gpa stack variable was prematurely released, resulting in the pointer pointing to it being an invalid pointer. In this case, the behavior when using the pointer becomes unpredictable.

6

When a UAF occurs, everything is “coincidental”, and any change in the size of a structure may lead to a change in the running result. You might catch the error, or you might not, and that’s what’s terrible about it.

1 Like