Segmentation fault when use GPA

Help
1

I am a newcomer to zig. I wrote such a piece of code. The main function is to loop http request paging data. I found that when I use gpa, it will report the following error：

Segmentation fault at address 0x1fee06b0010
E:\scoop\global\apps\zig-dev\current\lib\std\comptime_string_map.zig:79:28: 0x7ff6ca7575f0 in get (exchange2.exe.obj)
                if (mem.eql(u8, kv.key, str))
                           ^
E:\scoop\global\apps\zig-dev\current\lib\std\http\Client.zig:993:38: 0x7ff6ca75293c in request (exchange2.exe.obj)
    const protocol = protocol_map.get(uri.scheme) orelse return error.UnsupportedUrlScheme;
                                     ^
E:\code\me\test-zig\exchange2.zig:76:41: 0x7ff6ca7520f3 in main (exchange2.exe.obj)
            var req = try client.request(.GET, uri.?, headers, .{});
                                        ^
E:\scoop\global\apps\zig-dev\current\lib\std\start.zig:378:65: 0x7ff6ca7555bc in WinStartup (exchange2.exe.obj)
    std.os.windows.kernel32.ExitProcess(initEventLoopAndCallMain());
                                                                ^
???:?:?: 0x7ffcd64926ac in ??? (KERNEL32.DLL)
???:?:?: 0x7ffcd754aa67 in ??? (ntdll.dll)
  • code
pub fn main() !void {
    // var arena = heap.ArenaAllocator.init(heap.page_allocator);
    // defer arena.deinit();
    // const allocator = arena.allocator();

    var gpa = heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    // init client
    var client = http.Client{
        .allocator = allocator,
    };
    defer client.deinit();

    var i: usize = 0;
    while (i < 6) : (i += 1) {
        // request params
        var uri: ?Uri = try Uri.parse("https://pokeapi.co/api/v2/pokemon/");
        var headers = http.Headers.init(allocator);
        defer headers.deinit();
        const response_body_max_len = 64 * 1024;

        while (uri != null) {
            // var _arena = heap.ArenaAllocator.init(heap.page_allocator);
            // defer _arena.deinit();
            // const _allocator = arena.allocator();

            // init request
            var req = try client.request(.GET, uri.?, headers, .{});
            defer req.deinit();
            // start request
            try req.start();
            // wait request
            try req.wait();

            // handle response
            const response_status = req.response.status;
            switch (response_status) {
                .ok => {
                    var slice = try req.reader().readAllAlloc(allocator, response_body_max_len);
                    defer allocator.free(slice);

                    // parse json
                    const parse_options = json.ParseOptions{
                        .ignore_unknown_fields = true,
                        .allocate = .alloc_always,
                    };

                    var parsed = try json.parseFromSlice(PokemonResult(PokemonItem), allocator, slice, parse_options);
                    defer parsed.deinit();
                    const pokemon_result = parsed.value;

                    // log.debug("pokemon_result: {any}", .{pokemon_result});

                    const next_uri = pokemon_result.next;
                    log.info("next_uri: {?s}", .{next_uri});
                    uri = if (next_uri == null) null else try Uri.parse(next_uri.?);
                },
                else => |s| {
                    log.err("get http status: {any}", .{s});
                    uri = null;
                },
            }
        }
    }

    std.time.sleep(1000 * std.time.ns_per_s);
}
2

This is the important bit of code.

var parsed = try json.parseFromSlice(PokemonResult(PokemonItem), allocator, slice, parse_options);
defer parsed.deinit();
const pokemon_result = parsed.value;

// log.debug("pokemon_result: {any}", .{pokemon_result});

const next_uri = pokemon_result.next;
log.info("next_uri: {?s}", .{next_uri});
uri = if (next_uri == null) null else try Uri.parse(next_uri.?);

Uri.parse keeps a reference to the string that was passed into it.

However as soon as the end of scope is reached, that string is deallocated(you called defer parsed.deinit() where parsed is owning the next_uri string)

So when the next iteration of the loop starts you are reading invalid memory, causing a Segmentation fault.

To solve this you need to make a local copy of the string and make sure that it gets freed. I would do something like this:

        var uri_string: ?[]const u8 = try allocator.dupe(u8, "https://pokeapi.co/api/v2/pokemon/"); // Duping the original string as well to simplify the code
        ...
        while (uri_string != null) {
            defer allocator.free(uri_string.?); // Don't forget to free it.
            const uri = try Uri.parse(uri_string.?); // Doing this at the beginning of the loop reduces code duplication
            ...
            switch (response_status) {
                .ok => {
                    ...
                    uri_string = try allocator.dupe(next_uri.?); // Duplicate the string to avoid segfault
                },
                else => |s| {
                    ...
                    uri_string = null; // Why not use a `break` here by the way?
                },
            }
2 Likes
3

Thanks for the detailed answer, I also wonder why I don’t have this problem with other allocators？

4

Thanks for the detailed answer, I also wonder why I don’t have this problem with other allocators？

The GeneralPurposeAllocator is designed to make catching use-after-free errors easier in debug mode. Other allocators may just write something else into the freed memory without you noticing. This is undefined behavior. You just got lucky with the other allocators, likely because you didn’t do any other allocations in the mean-time.

6

Also, arena allocator won’t deallocate parsed until you .deinit the allocator

1 Like
7

There are still errors in the modified code, where did I copy it wrong?

╰─❯ zig run exchange2.zig 
info: next_uri: https://pokeapi.co/api/v2/pokemon/?offset=20&limit=20
error(gpa): Double free detected. Allocation:
E:\scoop\global\apps\zig-dev\current\lib\std\mem\Allocator.zig:319:40: 0x7ff7fa7d1057 in dupe__anon_5546 (exchange2.exe.obj)
    const new_buf = try allocator.alloc(T, m.len);
                                       ^
E:\code\me\test-zig\exchange2.zig:72:81: 0x7ff7fa7d2ae6 in main (exchange2.exe.obj)
                    uri_str = if (next_uri == null) null else try allocator.dupe(u8, next_uri.?);
                                                                                ^
E:\scoop\global\apps\zig-dev\current\lib\std\start.zig:378:65: 0x7ff7fa7d5e4c in WinStartup (exchange2.exe.obj)
    std.os.windows.kernel32.ExitProcess(initEventLoopAndCallMain());
                                                                ^
???:?:?: 0x7ffcd64926ac in ??? (KERNEL32.DLL)
 First free:
E:\code\me\test-zig\exchange2.zig:36:33: 0x7ff7fa7d2c2b in main (exchange2.exe.obj)
            defer allocator.free(uri_str.?);
                                ^
E:\scoop\global\apps\zig-dev\current\lib\std\start.zig:378:65: 0x7ff7fa7d5e4c in WinStartup (exchange2.exe.obj)
    std.os.windows.kernel32.ExitProcess(initEventLoopAndCallMain());
                                                                ^
???:?:?: 0x7ffcd64926ac in ??? (KERNEL32.DLL)
???:?:?: 0x7ffcd754aa67 in ??? (ntdll.dll)
 Second free:
E:\code\me\test-zig\exchange2.zig:36:33: 0x7ff7fa7d24dc in main (exchange2.exe.obj)
            defer allocator.free(uri_str.?);
                                ^
E:\scoop\global\apps\zig-dev\current\lib\std\start.zig:378:65: 0x7ff7fa7d5e4c in WinStartup (exchange2.exe.obj)
    std.os.windows.kernel32.ExitProcess(initEventLoopAndCallMain());
                                                                ^
???:?:?: 0x7ffcd64926ac in ??? (KERNEL32.DLL)
???:?:?: 0x7ffcd754aa67 in ??? (ntdll.dll)

error: UnexpectedCharacter
E:\scoop\global\apps\zig-dev\current\lib\std\Uri.zig:276:13: 0x7ff7fa7d2e78 in parse (exchange2.exe.obj)
            return error.UnexpectedCharacter;
            ^
E:\code\me\test-zig\exchange2.zig:42:25: 0x7ff7fa7d2500 in main (exchange2.exe.obj)
            const uri = try Uri.parse(uri_str.?);


const std = @import("std");
const log = std.log;
const mem = std.mem;
const heap = std.heap;
const io = std.io;
const fs = std.fs;
const process = std.process;
const http = std.http;
const Uri = std.Uri;
const json = std.json;
const builtin = @import("builtin");

pub fn main() !void {
    // var arena = heap.ArenaAllocator.init(heap.page_allocator);
    // defer arena.deinit();
    // const allocator = arena.allocator();

    var gpa = heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    // init client
    var client = http.Client{
        .allocator = allocator,
    };
    defer client.deinit();

    var i: usize = 0;
    while (i < 6) : (i += 1) {
        // request params
        var uri_str: ?[]const u8 = try allocator.dupe(u8, "https://pokeapi.co/api/v2/pokemon/");
        var headers = http.Headers.init(allocator);
        defer headers.deinit();
        const response_body_max_len = 64 * 1024;

        while (uri_str != null) {
            defer allocator.free(uri_str.?);
            // var _arena = heap.ArenaAllocator.init(heap.page_allocator);
            // defer _arena.deinit();
            // const _allocator = arena.allocator();

            // init request
            const uri = try Uri.parse(uri_str.?);
            var req = try client.request(.GET, uri, headers, .{});
            defer req.deinit();
            // start request
            try req.start();
            // wait request
            try req.wait();

            // handle response
            const response_status = req.response.status;
            switch (response_status) {
                .ok => {
                    var slice = try req.reader().readAllAlloc(allocator, response_body_max_len);
                    defer allocator.free(slice);

                    // parse json
                    const parse_options = json.ParseOptions{
                        .ignore_unknown_fields = true,
                        .allocate = .alloc_always,
                    };

                    var parsed = try json.parseFromSlice(PokemonResult(PokemonItem), allocator, slice, parse_options);
                    defer parsed.deinit();
                    const pokemon_result = parsed.value;

                    // log.debug("pokemon_result: {any}", .{pokemon_result});

                    const next_uri = pokemon_result.next;
                    log.info("next_uri: {?s}", .{next_uri});

                    uri_str = if (next_uri == null) null else try allocator.dupe(u8, next_uri.?);
                },
                else => |s| {
                    log.err("get http status: {any}", .{s});
                    uri_str = null;
                },
            }
        }
    }

    std.time.sleep(1000 * std.time.ns_per_s);
}

const PokemonItem = struct {
    name: []const u8,
    url: []const u8,
};

fn PokemonResult(comptime T: type) type {
    return struct {
        count: u32,
        next: ?[]const u8,
        previous: ?[]const u8,
        results: []const T,
    };
}
8

Oh, my bad. Here, try this:

        var uri_string: ?[]const u8 = try allocator.dupe(u8, "https://pokeapi.co/api/v2/pokemon/"); // Duping the original string as well to simplify the code
        ...
        while (uri_string != null) {
            const uri = try Uri.parse(uri_string.?); // Doing this at the beginning of the loop reduces code duplication
            ...
            switch (response_status) {
                .ok => {
                    ...
                    allocator.free(uri_string.?); // Need to free it before changing it.
                    uri_string = try allocator.dupe(next_uri.?); // Duplicate the string to avoid segfault
                },
                else => |s| {
                    ...
                    allocator.free(uri_string.?); // Need to free it before changing it.
                    uri_string = null; // Why not use a `break` here by the way?
                },
            }
2 Likes
9

That’s not always true. Arena allocators sometimes deallocate memory if the freed memory is at the end of the underlying data structure(which happens if you are freeing the last allocation). Check out the code of ArenaAllocator.free

3 Likes
10

It works fine, thanks a lot for your help, it would be great if you can provide learning materials related to memory allocator :blush:

11

Oh, thanks, I didn’t know that. Will definitely check out the implementation.