Skip to content

Fix GeneralPurposeAllocator crash when deallocating metadata#20000

Merged
andrewrk merged 2 commits intoziglang:masterfrom
Frojdholm:fix-gpa-crash-when-deallocating-metadata
Jun 6, 2024
Merged

Fix GeneralPurposeAllocator crash when deallocating metadata#20000
andrewrk merged 2 commits intoziglang:masterfrom
Frojdholm:fix-gpa-crash-when-deallocating-metadata

Conversation

@Frojdholm
Copy link
Contributor

@Frojdholm Frojdholm commented May 19, 2024

Fixes #19977

This makes the following test not crash

const std = @import("std");

test "retain metadata and never unmap" {
    var gpa = std.heap.GeneralPurposeAllocator(.{
        .safety = true,
        .never_unmap = true,
        .retain_metadata = true,
    }){};
    defer std.debug.assert(gpa.deinit() == .ok);
    const allocator = gpa.allocator();

    const alloc = try allocator.alloc(u8, 8);
    allocator.free(alloc);

    const alloc2 = try allocator.alloc(u8, 8);
    allocator.free(alloc2);
}

The getMin/getMax -> remove pattern could be encapsulated in a function popMin/popMax that return the removed key.

@Frojdholm Frojdholm marked this pull request as ready for review May 19, 2024 21:52
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test case. Can it be added to the file as a unit test?

@Frojdholm
Copy link
Contributor Author

Nice test case. Can it be added to the file as a unit test?

Sure, added 👍

However, I was a bit confused that the "double frees" test wasn't already failing since it's using the same setup, but with a backing allocator of another GPA.

@Frojdholm Frojdholm requested a review from andrewrk May 20, 2024 16:36
@Frojdholm
Copy link
Contributor Author

Test failure was a flaky windows test

@andrewrk andrewrk merged commit 3964b2a into ziglang:master Jun 6, 2024
@andrewrk
Copy link
Member

andrewrk commented Jun 6, 2024

Thanks!

@Frojdholm Frojdholm deleted the fix-gpa-crash-when-deallocating-metadata branch June 6, 2024 07:20
andrewrk added a commit that referenced this pull request Jun 6, 2024
…ing-metadata

Fix GeneralPurposeAllocator crash when deallocating metadata
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GeneralPurposeAllocator: assertion failure when retain_metadata and never_unmap are used

2 participants