From 9ea29dc5085a060c18d0288d2c72d391d5279269 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Feb 2025 08:47:10 -0800 Subject: [PATCH] termio: free envmap when able, fix memory leak Caused by #5650 I actually don't understand how this didn't happen before or why we didn't notice it but it seems like the envmap was never freed. In the latest debug builds prior to this build GPA reports the leak. We should free the envmap when the subprocess is deinitialized. But also we can free the env map as soon as we start the subprocess which saves some small amount of memory at runtime. Additionally, we should only be freeing the envmap on error if we created it. --- src/termio/Exec.zig | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index d9730b970..caef2229d 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -704,7 +704,7 @@ const Subprocess = struct { arena: std.heap.ArenaAllocator, cwd: ?[]const u8, - env: EnvMap, + env: ?EnvMap, args: [][]const u8, grid_size: renderer.GridSize, screen_size: renderer.ScreenSize, @@ -725,7 +725,7 @@ const Subprocess = struct { // Get our env. If a default env isn't provided by the caller // then we get it ourselves. var env = cfg.env orelse try internal_os.getEnvMap(alloc); - errdefer env.deinit(); + errdefer if (cfg.env == null) env.deinit(); // If we have a resources dir then set our env var if (cfg.resources_dir) |dir| { @@ -1054,6 +1054,7 @@ const Subprocess = struct { pub fn deinit(self: *Subprocess) void { self.stop(); if (self.pty) |*pty| pty.deinit(); + if (self.env) |*env| env.deinit(); self.arena.deinit(); self.* = undefined; } @@ -1136,7 +1137,7 @@ const Subprocess = struct { var cmd: Command = .{ .path = self.args[0], .args = self.args, - .env = &self.env, + .env = if (self.env) |*env| env else null, .cwd = cwd, .stdin = if (builtin.os.tag == .windows) null else .{ .handle = pty.slave }, .stdout = if (builtin.os.tag == .windows) null else .{ .handle = pty.slave }, @@ -1170,6 +1171,12 @@ const Subprocess = struct { _ = posix.close(pty.slave); } + // Successful start we can clear out some memory. + if (self.env) |*env| { + env.deinit(); + self.env = null; + } + self.command = cmd; return switch (builtin.os.tag) { .windows => .{