`execOpts` causes memory leak

[sdykae]

I need to understand how can i use execOpts without having memory leak I was using queryOpts with a insert query, and everything were fine, I tried to migrate to use execOpts and get more details on errors, but I've got memory leaks on the same code that was ok with queryOpts

the code is simple

I have a zap service

pub const AccountService = struct {
    const Self = @This();
    allocator: std.mem.Allocator,
    databaseConnectionService: *DatabaseConnectionService,
    id: u32 = 0,
    pub fn init(allocator: std.mem.Allocator, databaseConnectionService: *DatabaseConnectionService) *Self {
        const instance = allocator.create(Self) catch |err| {
            std.debug.print("Failed to create instance: {}\n", .{err});
            std.process.exit(1);
        };
        instance.* = Self{
            .allocator = allocator,
            .databaseConnectionService = databaseConnectionService,
            .id = tools.generateUniqueId(),
        };
        return instance;
    }

    pub fn create(self: *Self, createAccountDto: CreateAccountDto) ?[]const u8 {
        const query = "INSERT INTO accounts (username, password) VALUES ($1, $2);";
        const result = self.databaseConnectionService.getPool().execOpts(query, .{ createAccountDto.username, createAccountDto.password }, .{ .column_names = false }) catch |err| {
            return self.databaseConnectionService.handleQueryError(err);  // LEAK IS HERE
        };
        std.debug.print("Result: {any}\n", .{result});
        return null;
    }

    pub fn login(self: *Self, loginAccountDto: LoginAccountDto) ?[]const u8 {
        const query = "SELECT username, password FROM accounts WHERE username = $1 AND password = $2;";
        const result = self.databaseConnectionService.getPool().queryOpts(query, .{ loginAccountDto.username, loginAccountDto.password }, .{ .column_names = false }) catch |err| {
            return self.databaseConnectionService.handleQueryError(err);
        };
        defer result.deinit();
        std.debug.print("Result: {any}\n", .{result});
        return null;
    }

    pub fn deinit(self: *Self) void {
        self.allocator.destroy(self);
    }
};

the functions are:

    pub fn handleQueryError(self: *Self, err: anyerror) ?[]const u8 {
        // If the error is a PostgreSQL error, get the detailed error information
        if (err == error.PG) {
            const conn = self.pool.acquire() catch |e| {
                std.debug.print("Failed to acquire connection: {}\n", .{e});
                return null;
            };
            // defer self.pool.release(conn);
            if (conn.err) |pgErr| {
                return tools.convertToJsonString(pgErr, self.allocator);
            } else {
                return tools.convertToJsonString(err, self.allocator);
            }
        } else {
            return tools.convertToJsonString(err, self.allocator);
        }
    }

pub fn convertToJsonString(obj: anytype, allocator: std.mem.Allocator) ?[]const u8 {
    var jsonString = std.ArrayList(u8).init(allocator);
    defer jsonString.deinit();

    const options = std.json.StringifyOptions{ .emit_null_optional_fields = false };

    std.json.stringify(obj, options, jsonString.writer()) catch |err| {
        std.debug.print("Failed to stringify JSON: {}\n", .{err});
        return null;
    };
    const jsonStringSlice = jsonString.toOwnedSlice() catch |err| {
        std.debug.print("Failed to convert to owned slice: {}\n", .{err});
        return null;
    };

    return jsonStringSlice;
}

Error

➜  ms-simple-rest-api-di-bs git:(main) ✗ zbr
INFO: Listening on port 3000
INFO: Server is running 1 worker X 8 threads with facil.io 0.7.4 (kqueue)
* Detected capacity: 131056 open file limit
* Root pid: 58811
* Press ^C to stop

::1 - - [Wed, 24 Jul 2024 02:28:21 GMT] "POST /register HTTP/1.1" 400 328b 9341us
DTO: dto.create-account.dto.CreateAccountDto{ .username = { 117, 115, 101, 114, 49, 49 }, .password = { 80, 97, 36, 36, 119, 48, 114, 100 } }
^C
➜  ms-simple-rest-api-di-bs git:(main) ✗ INFO: (58811) detected exit signal.
INFO:    ---  Shutdown Complete  ---

error(gpa): memory address 0x105124c00 leaked:
/Users/sdykae/.zvm/0.13.0/lib/std/array_list.zig:457:67: 0x105035e7f in ensureTotalCapacityPrecise (ms-simple-rest-api-di-bs)
                const new_memory = try self.allocator.alignedAlloc(T, alignment, new_capacity);
                                                                  ^
/Users/sdykae/.zvm/0.13.0/lib/std/array_list.zig:434:51: 0x105031a0b in ensureTotalCapacity (ms-simple-rest-api-di-bs)
            return self.ensureTotalCapacityPrecise(better_capacity);
                                                  ^
/Users/sdykae/.zvm/0.13.0/lib/std/array_list.zig:468:44: 0x10502d7f3 in ensureUnusedCapacity (ms-simple-rest-api-di-bs)
            return self.ensureTotalCapacity(try addOrOom(self.items.len, additional_count));
                                           ^
/Users/sdykae/.zvm/0.13.0/lib/std/array_list.zig:305:42: 0x10503190f in appendSlice (ms-simple-rest-api-di-bs)
            try self.ensureUnusedCapacity(items.len);
                                         ^
/Users/sdykae/.zvm/0.13.0/lib/std/array_list.zig:358:33: 0x10502f337 in appendWrite (ms-simple-rest-api-di-bs)
            try self.appendSlice(m);
                                ^
/Users/sdykae/.zvm/0.13.0/lib/std/io.zig:360:27: 0x1050215ef in typeErasedWriteFn (ms-simple-rest-api-di-bs)
            return writeFn(ptr.*, bytes);
                          ^

debug: Has leaked: true

[karlseguin]

Do you have a full stack of the leak? Maybe if you run with -freference-trace. Is the jsonStringSlice ever freed?

I'm not sure how you're conclusing that execOpts is leaking even though you're saying the leak is happening after execOpts terminates. I might be missing something, but that seems like a leap.

While I don't think it's related to the leak, your handleQueryError doesn't seem correct to me - there's no guarantee that the connection you get from the pool in handleQueryError is the same error connection that caused the error when you called execOpts. If you care about the error, you need to explicitly get a connection. Something like:

var conn = try pool.acquire();
defer conn.release();
conn.execOpts(....) catch |err| {
  return handleError(err, &conn)
}
...

[sdykae]

Thank you for the insight, somehow I needed to free the json string ([]const u8) when execOpts is used, and not with queryOpts