ziglang/zig

Memory leaks or not?

AssertionBit opened this issue · 2 comments

Zig Version

0.12.0

Steps to Reproduce and Observed Behavior

No matter what to do here, passed allocator will cause neither not free memory or segmentation fault. Using errdefer is causing mostly segmentation error, which means for me, that there is nothing I can do.

Use this code in any file:

const std = @import("std");

pub const CellPrimitiveType = enum(u8) {
    Char = 0x1,
    String = 0x2,
    Integer = 0x3,
    Long = 0x4,
    Binary = 0x5,
    ForeignIndex = 0x6,
};

pub const CellAttributes = enum(u8) {
    Index = 0x1,
    NotNull = 0x2,
    Nullable = 0x3,
    Unique = 0x4,
    NonUnique = 0x5,
};

/// @brief Cell initialization error
///
/// Error appearing when creating new cell entity.
pub const CellInitError = error{
    EmptyDataSet,
    NameMissing,
    TypeDefinitionMissing,
    AttributeIsMissing,
    AllocationResizeRefused,
};

/// @brief Collection of errors related to collisions
///
/// This set of errors contains all elements that possibly might collisions between values.
pub const CellCollisionError = error{
    AttributeExists,
    AttributePermissionError,
    IndexExists,
};

/// @brief Single cell element
/// @author Mark Sorokin <assertionbit@icloud.com>
///
/// Cell is structure containging single element of table. It might be collected into row
/// where they will hold data or be in table, representing single data type.
pub const Cell = struct {
    alloc: *const std.mem.Allocator,
    cell_name: []u8,
    cell_type: CellPrimitiveType,
    cell_attributes: []CellAttributes,
    value: []u8,

    /// @brief Initiate cell with it's definition
    /// @param alloc - Pointer to used allocator
    /// @param data - Array of data which represents initial definition, without null-terminator
    /// @return Cell definition element, without data
    /// @throws Error{OutOfMemory} - When out of memory
    /// @throws CellInitError - When provided data is not correct
    pub fn init(alloc: *const std.mem.Allocator, data: []u8) !*const Cell {
        // Data must not be null
        if (data.len == 0) {
            return CellInitError.EmptyDataSet;
        }

        // Name of cell
        var cell_name: []u8 = try alloc.alloc(u8, data.len);
        var needToBreak = false;
        var name_size: usize = 0;
        while ((name_size < data.len) and (!needToBreak)) : (name_size += 1) {
            if (((data[name_size] == 0x0) or (data[name_size] <= 0x6))) {
                needToBreak = true;
                continue;
            }

            cell_name[name_size] = data[name_size];
        }
        if (!alloc.resize(cell_name, name_size - 1)) {
            return CellInitError.AllocationResizeRefused;
        }

        if (name_size - 1 == 0) {
            return CellInitError.NameMissing;
        }

        if (cell_name.len == data.len) {
            alloc.free(cell_name);
            return CellInitError.TypeDefinitionMissing;
        }

        const cell_type: CellPrimitiveType = @enumFromInt(data[cell_name.len + 1]);
        var cell_attributes: []CellAttributes = try alloc.alloc(
            CellAttributes,
            cell_name.len - 1 - 1,
        );

        if (cell_attributes.len == 0) {
            cell_attributes = try alloc.alloc(CellAttributes, 2);
            cell_attributes[0] = CellAttributes.Nullable;
            cell_attributes[1] = CellAttributes.NonUnique;

            return &Cell{
                .alloc = alloc,
                .cell_name = cell_name,
                .cell_type = cell_type,
                .cell_attributes = cell_attributes,
                .value = undefined,
            };
        }

        for (data, (cell_name.len + 1)..) |value, index| {
            cell_attributes[index - cell_attributes.len] = @enumFromInt(value);
        }

        return &Cell{
            .alloc = alloc,
            .cell_name = cell_name,
            .cell_type = cell_type,
            .cell_attributes = cell_attributes,
            .value = undefined,
        };
    }

    pub fn deinit(this: *const Cell) void {
        this.alloc.free(this.cell_name);
        this.alloc.free(this.cell_attributes);
        if ((this.cell_name.len != 0)) {
            this.alloc.free(this.value);
        }
    }

    /// @brief Get byte definition of cell
    /// @param this - Pointer to original cell element
    /// @return Null-terminated byte array containging difinition of cell
    /// @apiNote Return value must be deallocated
    /// @throws Error{OutOfMemory} when machine reaches memory limit
    ///
    /// Method returns byte definition of cell which might be used for writing
    /// table definition. It helps internals to understand what is actual value
    /// to expect.
    pub fn getDefinition(this: *const Cell) ![]u8 {
        var definition: []u8 = try this.alloc.alloc(u8, this.cell_name + 1 + this.cell_attributes.len + 1);

        // Injecting name of value
        for (this.cell_name, 0..) |char, index| {
            definition[index] = char;
        }

        definition[this.cell_name.len + 1] = this.cell_type;

        // Injecting attributes
        for (this.cell_attributes, 0..) |value, index| {
            definition[this.cell_name + 1 + index] = value;
        }

        // Null-terminator
        definition[definition.len - 1] = 0x0;

        return definition;
    }

    pub fn addAttribute(this: *const Cell) CellCollisionError!void {
        _ = this;
    }
};

test "Test empty data set" {
    var dataset = [_]u8{};
    const errorValue = Cell.init(&std.testing.allocator, &dataset);
    try std.testing.expectError(CellInitError.EmptyDataSet, errorValue);
}

test "Testing failing name" {
    var dataset = [_]u8{
        0x0, 0x1, 0x2, 0x3,
    };
    const errorValue = Cell.init(&std.testing.allocator, &dataset);
    try std.testing.expectError(CellInitError.NameMissing, errorValue);
}

test "Test missing attributes" {
    var dataset = [_]u8{ 'E', 'E', 0x1 };

    const result = try Cell.init(&std.testing.allocator, &dataset);
    defer result.deinit();
}

test "Testing full cell" {
    var dataset = [_]u8{ 'E', 'E', 0x1 };

    const result = try Cell.init(&std.testing.allocator, &dataset);
    defer result.deinit();
}

Expected Behavior

Using errdefer in init I may free memory, which was used during runtime. Either deinit must free memory, but it doesnt' work neither.

System info about this bug:

Darwin 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:12:41 PDT 2024; root:xnu-10063.101.17 arm64
  • return &Cell{ is returning a pointer to stack memory, which goes out-of-scope when the function returns. If you want to return a pointer to Cell, you'll need to heap allocate the memory for the Cell.
  • Side note: you shouldn't be passing/storing std.mem.Allocator as a pointer, you should accept/store a std.mem.Allocator value (so Cell.init(std.testing.allocator, &dataset) and alloc: std.mem.Allocator,)

In the future, ask for help in one of the community spaces before turning to the issue tracker. Closing this, please open a new issue if there ends up being a Zig bug involved here.