Skip to content

Commit fac9011

Browse files
committed
Fix GPA memory leak: use arena allocator for bootstrap data
Protocol registration, XML keywords, and IO default streams were allocated via GPA but never freed (91 leaks). These are permanent bootstrap data, so use a dedicated arena allocator that is freed on process exit.
1 parent ea7b995 commit fac9011

5 files changed

Lines changed: 63 additions & 21 deletions

File tree

‎src/cache_gen.zig‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const registry = @import("lang/registry.zig");
2121
const bootstrap = @import("engine/bootstrap.zig");
2222
const gc_mod = @import("runtime/gc.zig");
2323
const keyword_intern = @import("runtime/keyword_intern.zig");
24+
const clojure_core_protocols = @import("lang/lib/clojure_core_protocols.zig");
2425

2526
pub fn main() !void {
2627
var gpa: std.heap.GeneralPurposeAllocator(.{}) = .init;
@@ -43,6 +44,9 @@ pub fn main() !void {
4344
keyword_intern.init(allocator);
4445
defer keyword_intern.deinit();
4546

47+
clojure_core_protocols.initArena(allocator);
48+
defer clojure_core_protocols.deinitArena();
49+
4650
// Bootstrap from source
4751
var env = Env.init(allocator);
4852
defer env.deinit();

‎src/lang/lib/clojure_core_protocols.zig‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,31 @@ const predicates = @import("../builtins/predicates.zig");
2626
const registry = @import("../registry.zig");
2727
const NamespaceDef = registry.NamespaceDef;
2828

29+
// Arena allocator for protocol registration data.
30+
// Protocol data (Protocol, PersistentArrayMap, HeapString, ProtocolFn, Value arrays)
31+
// lives for the entire process lifetime. Using an arena allows bulk deallocation
32+
// at shutdown, preventing GPA leak reports without individual tracking.
33+
var protocol_arena: ?std.heap.ArenaAllocator = null;
34+
35+
/// Initialize the protocol arena. Must be called before any protocol registration.
36+
pub fn initArena(backing: Allocator) void {
37+
protocol_arena = std.heap.ArenaAllocator.init(backing);
38+
}
39+
40+
/// Free all protocol registration data. Called at shutdown.
41+
pub fn deinitArena() void {
42+
if (protocol_arena) |*arena| {
43+
arena.deinit();
44+
protocol_arena = null;
45+
}
46+
}
47+
48+
/// Get the protocol arena allocator. Falls back to the given allocator if arena not initialized.
49+
pub fn getArenaAllocator(fallback: Allocator) Allocator {
50+
if (protocol_arena) |*arena| return arena.allocator();
51+
return fallback;
52+
}
53+
2954
// ============================================================
3055
// Protocol method implementations (extend-type functions)
3156
// ============================================================
@@ -151,8 +176,9 @@ pub fn createProtocol(
151176
sigs: []const MethodSig,
152177
extend_via_meta: bool,
153178
) !*Protocol {
154-
const protocol = try allocator.create(Protocol);
155-
const empty_map = try allocator.create(PersistentArrayMap);
179+
const alloc = getArenaAllocator(allocator);
180+
const protocol = try alloc.create(Protocol);
181+
const empty_map = try alloc.create(PersistentArrayMap);
156182
empty_map.* = .{ .entries = &.{} };
157183
protocol.* = .{
158184
.name = name,
@@ -170,7 +196,7 @@ pub fn createProtocol(
170196
var i: usize = 0;
171197
while (i < sigs.len) {
172198
const method_name = sigs[i].name;
173-
const pf = try allocator.create(ProtocolFn);
199+
const pf = try alloc.create(ProtocolFn);
174200
pf.* = .{
175201
.protocol = protocol,
176202
.method_name = method_name,
@@ -195,27 +221,29 @@ pub fn extendType(
195221
type_key: []const u8,
196222
methods: []const struct { name: []const u8, func: *const fn (Allocator, []const Value) anyerror!Value },
197223
) !void {
224+
const alloc = getArenaAllocator(allocator);
198225
// Build method map: [name1, fn1, name2, fn2, ...]
199-
const method_entries = try allocator.alloc(Value, methods.len * 2);
226+
const method_entries = try alloc.alloc(Value, methods.len * 2);
200227
for (methods, 0..) |m, i| {
201-
method_entries[i * 2] = Value.initString(allocator, m.name);
228+
method_entries[i * 2] = Value.initString(alloc, m.name);
202229
method_entries[i * 2 + 1] = Value.initBuiltinFn(m.func);
203230
}
204-
const method_map = try allocator.create(PersistentArrayMap);
231+
const method_map = try alloc.create(PersistentArrayMap);
205232
method_map.* = .{ .entries = method_entries };
206233

207234
// Add to impls: grow the impls map
208235
const old_impls = protocol.impls;
209236
const old_entries = old_impls.entries;
210-
const new_entries = try allocator.alloc(Value, old_entries.len + 2);
237+
const new_entries = try alloc.alloc(Value, old_entries.len + 2);
211238
@memcpy(new_entries[0..old_entries.len], old_entries);
212-
new_entries[old_entries.len] = Value.initString(allocator, type_key);
239+
new_entries[old_entries.len] = Value.initString(alloc, type_key);
213240
new_entries[old_entries.len + 1] = Value.initMap(method_map);
214-
const new_impls = try allocator.create(PersistentArrayMap);
241+
const new_impls = try alloc.create(PersistentArrayMap);
215242
new_impls.* = .{ .entries = new_entries };
216-
// Free old impls (entries Values are copied by value, still referenced from new_entries)
217-
if (old_entries.len > 0) allocator.free(old_entries);
218-
allocator.destroy(old_impls);
243+
// With arena allocator, individual frees are no-ops (arena frees everything at once).
244+
// Keep the free calls for correctness when used without arena.
245+
if (old_entries.len > 0) alloc.free(old_entries);
246+
alloc.destroy(old_impls);
219247
protocol.impls = new_impls;
220248
protocol.generation +%= 1;
221249
}

‎src/lang/lib/clojure_java_io.zig‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,17 +256,19 @@ fn registerProtocols(allocator: Allocator, env: *Env) anyerror!void {
256256

257257
fn makeDefaultStreamsImpl(allocator: Allocator) !Value {
258258
// Build a PersistentArrayMap directly (no callCore at registration time)
259+
// Use protocol arena for permanent bootstrap allocations.
260+
const alloc = clojure_core_protocols.getArenaAllocator(allocator);
259261
const runtime_collections = @import("../../runtime/collections.zig");
260-
const entries = try allocator.alloc(Value, 8);
261-
entries[0] = Value.initKeyword(allocator, .{ .ns = null, .name = "make-reader" });
262+
const entries = try alloc.alloc(Value, 8);
263+
entries[0] = Value.initKeyword(alloc, .{ .ns = null, .name = "make-reader" });
262264
entries[1] = Value.initBuiltinFn(&defaultMakeReaderFn);
263-
entries[2] = Value.initKeyword(allocator, .{ .ns = null, .name = "make-writer" });
265+
entries[2] = Value.initKeyword(alloc, .{ .ns = null, .name = "make-writer" });
264266
entries[3] = Value.initBuiltinFn(&defaultMakeWriterFn);
265-
entries[4] = Value.initKeyword(allocator, .{ .ns = null, .name = "make-input-stream" });
267+
entries[4] = Value.initKeyword(alloc, .{ .ns = null, .name = "make-input-stream" });
266268
entries[5] = Value.initBuiltinFn(&defaultMakeInputStreamFn);
267-
entries[6] = Value.initKeyword(allocator, .{ .ns = null, .name = "make-output-stream" });
269+
entries[6] = Value.initKeyword(alloc, .{ .ns = null, .name = "make-output-stream" });
268270
entries[7] = Value.initBuiltinFn(&defaultMakeOutputStreamFn);
269-
const map = try allocator.create(runtime_collections.PersistentArrayMap);
271+
const map = try alloc.create(runtime_collections.PersistentArrayMap);
270272
map.* = .{ .entries = entries };
271273
return Value.initMap(map);
272274
}

‎src/lang/lib/clojure_xml.zig‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,14 +375,17 @@ fn emitFn(allocator: Allocator, args: []const Value) anyerror!Value {
375375
/// Post-registration: bind tag/attrs/content to their keyword values.
376376
fn postRegisterImpl(allocator: Allocator, ns: anytype) void {
377377
// tag = :tag, attrs = :attrs, content = :content
378+
// Use protocol arena for permanent bootstrap allocations.
379+
const clojure_core_protocols = @import("clojure_core_protocols.zig");
380+
const alloc = clojure_core_protocols.getArenaAllocator(allocator);
378381
if (ns.mappings.get("tag")) |v| {
379-
v.bindRoot(kw(allocator, "tag"));
382+
v.bindRoot(kw(alloc, "tag"));
380383
}
381384
if (ns.mappings.get("attrs")) |v| {
382-
v.bindRoot(kw(allocator, "attrs"));
385+
v.bindRoot(kw(alloc, "attrs"));
383386
}
384387
if (ns.mappings.get("content")) |v| {
385-
v.bindRoot(kw(allocator, "content"));
388+
v.bindRoot(kw(alloc, "content"));
386389
}
387390
}
388391

‎src/main.zig‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const lifecycle = @import("runtime/lifecycle.zig");
2323
const runner = @import("app/runner.zig");
2424
const cli = @import("app/cli.zig");
2525
const test_runner = @import("app/test_runner.zig");
26+
const clojure_core_protocols = @import("lang/lib/clojure_core_protocols.zig");
2627

2728
pub fn main() !void {
2829
var gpa: std.heap.GeneralPurposeAllocator(.{}) = .init;
@@ -45,6 +46,10 @@ pub fn main() !void {
4546
keyword_intern.init(allocator);
4647
defer keyword_intern.deinit();
4748

49+
// Initialize protocol arena (for protocol registration data)
50+
clojure_core_protocols.initArena(allocator);
51+
defer clojure_core_protocols.deinitArena();
52+
4853
// Initialize load path infrastructure for require/load
4954
ns_ops.init(allocator);
5055
defer ns_ops.deinit();

0 commit comments

Comments
 (0)