Snektron / vulkan-zig

Vulkan binding generator for Zig
MIT License
519 stars 60 forks source link

gpa detects memory leak #162

Closed griush closed 2 months ago

griush commented 2 months ago

While working on an mini project, using the gpa, on deinit it detects memory leaks on all function calls that take in an allocator. I'm not sure if it's me that's doing something wrong (I made sure I'm freeing the returned memory) or it's actually a leak.

Output:

error(gpa): memory address 0x22545430000 leaked: 
N:\dev\forge\example\sandbox\.zig-cache\o\7d9b01588e4b13789f8eef7f0b346108\vk.zig:26152:45: 0xa35ac0 in getPhysicalDeviceSurfacePresentModesAllocKHR (sandbox.exe.obj)
                data = try allocator.realloc(data, count);
N:\dev\forge\example\sandbox\.zig-cache\o\7d9b01588e4b13789f8eef7f0b346108\vk.zig:26104:45: 0xa354a0 in getPhysicalDeviceSurfaceFormatsAllocKHR (sandbox.exe.obj)
                data = try allocator.realloc(data, count);
error(gpa): memory address 0x225454f0000 leaked: 
N:\dev\forge\example\sandbox\.zig-cache\o\7d9b01588e4b13789f8eef7f0b346108\vk.zig:32477:45: 0xa37790 in getSwapchainImagesAllocKHR (sandbox.exe.obj)
                data = try allocator.realloc(data, count);

and more.

poconn commented 2 months ago

Can you reproduce with the example in examples/? It currently discards the result of gpa.deinit(), but if you change to assert on it then it should pass:

diff --git a/examples/triangle.zig b/examples/triangle.zig
index ca205be..a1357ed 100644
--- a/examples/triangle.zig
+++ b/examples/triangle.zig
@@ -4,6 +4,7 @@ const c = @import("c.zig");
 const GraphicsContext = @import("graphics_context.zig").GraphicsContext;
 const Swapchain = @import("swapchain.zig").Swapchain;
 const Allocator = std.mem.Allocator;
+const assert = std.debug.assert;

 const vert_spv align(@alignOf(u32)) = @embedFile("vertex_shader").*;
 const frag_spv align(@alignOf(u32)) = @embedFile("fragment_shader").*;
@@ -64,7 +65,7 @@ pub fn main() !void {
     defer c.glfwDestroyWindow(window);

     var gpa = std.heap.GeneralPurposeAllocator(.{}){};
-    defer _ = gpa.deinit();
+    defer assert(gpa.deinit() == .ok);
     const allocator = gpa.allocator();

     const gc = try GraphicsContext.init(allocator, app_name, window);

(I made sure I'm freeing the returned memory)

If the above passes then I would double-check the details of how you're doing this, e.g. that you're using the same allocator and that the free()s are actually being called. Pretty sure the generated code for those functions is correct, I don't see how it could be leaking memory internally.

griush commented 2 months ago

pretty sure it's me that is doing something wrong, I have an idea of what it could be, still I checked with the debugger that the frees are being called

griush commented 2 months ago

yeah, I'm just stupid