From 411f6794917f4ff961636355731879f9aac26850 Mon Sep 17 00:00:00 2001 From: Ben Buhse Date: Fri, 6 Mar 2026 11:37:21 -0600 Subject: [PATCH] Fix bug where Buffers were never freed The issue was that, when reiniting Buffers, the intrusive linkedlist node was clobbered and we lost reference to it. That meant most of the Buffers would be memory leaks. Now, we save the node during reinitializion. --- docs/TODO.md | 1 + src/Buffer.zig | 9 +++++++++ src/BufferPool.zig | 9 ++++----- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/TODO.md b/docs/TODO.md index 1db84c0..e1bae66 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -2,6 +2,7 @@ These are in rough order of my priority, though no promises I do them in this order. +- [ ] Fix mouse resizing - [ ] Add gap support - [ ] Support window tag/order caching between WM restarts (within a river session) - [ ] Add build-time options for including the wallpaper (and maybe bar) diff --git a/src/Buffer.zig b/src/Buffer.zig index 8bba174..ba4b22a 100644 --- a/src/Buffer.zig +++ b/src/Buffer.zig @@ -76,6 +76,15 @@ pub fn init(shm: *wl.Shm, width: u31, height: u31) !Buffer { }; } +/// Re-initialize this buffer with new dimensions, preserving its position in +/// any intrusive linked list. Equivalent to deinit + init but keeps node intact. +pub fn reinit(buffer: *Buffer, shm: *wl.Shm, width: u31, height: u31) !void { + const saved_node = buffer.node; + buffer.deinit(); + buffer.* = try Buffer.init(shm, width, height); + buffer.node = saved_node; +} + pub fn deinit(buffer: *Buffer) void { _ = buffer.pixman_image.unref(); buffer.wl_buffer.destroy(); diff --git a/src/BufferPool.zig b/src/BufferPool.zig index a0c3454..a18f4e4 100644 --- a/src/BufferPool.zig +++ b/src/BufferPool.zig @@ -37,7 +37,7 @@ pub fn deinit(buffer_pool: *BufferPool) void { /// Get a buffer with the specified dimensions. If possible, an idle buffer is /// reused, otherwise a new one is created. pub fn nextBuffer(buffer_pool: *BufferPool, wl_shm: *wl.Shm, width: u31, height: u31) !*Buffer { - log.debug("looking for buffer with dimensions {}x{}, total existing buffers: {}", .{ width, height, buffer_pool.len }); + log.debug("Looking for buffer with dimensions {}x{}, total existing buffers: {}", .{ width, height, buffer_pool.len }); defer { // Clear up extra buffers if (buffer_pool.len > max_buffer_multiplicity * buffer_pool.surface_count) { @@ -71,8 +71,7 @@ fn findSuitableBuffer(buffer_pool: *BufferPool, wl_shm: *wl.Shm, width: u31, hei // No buffer has matching dimensions, however we do have an unbusy // buffer which we can just re-init. if (first_unbusy_buffer) |buffer| { - buffer.deinit(); - buffer.* = try Buffer.init(wl_shm, width, height); + try buffer.reinit(wl_shm, width, height); buffer.setListener(); return buffer; } @@ -81,7 +80,7 @@ fn findSuitableBuffer(buffer_pool: *BufferPool, wl_shm: *wl.Shm, width: u31, hei } fn newBuffer(buffer_pool: *BufferPool, wl_shm: *wl.Shm, width: u31, height: u31) !*Buffer { - log.debug("creating new buffer {}x{}", .{ width, height }); + log.debug("Creating new {}x{} buffer", .{ width, height }); const buffer = try utils.gpa.create(Buffer); errdefer utils.gpa.destroy(buffer); buffer.* = try Buffer.init(wl_shm, width, height); @@ -92,8 +91,8 @@ fn newBuffer(buffer_pool: *BufferPool, wl_shm: *wl.Shm, width: u31, height: u31) } fn cullBuffers(buffer_pool: *BufferPool) void { - log.debug("culling extra buffers", .{}); var overhead = buffer_pool.len - max_buffer_multiplicity * buffer_pool.surface_count; + log.debug("Culling extra buffers {d}->{d}", .{ buffer_pool.len, buffer_pool.surface_count }); var it = buffer_pool.buffers.first; while (it) |node| { if (overhead == 0) break;