From bc791b064a3e6139f23e935007ef8be08a8f1a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Tue, 9 Jun 2026 11:39:36 -0300 Subject: [PATCH 1/2] fix(flow): free runtime node catalog on App shutdown (#209) The editor leaked an allocation on exit whenever a project with a .labelle/flow_catalog.json sidecar was loaded. App.reloadFlowNodeCatalog installs a RuntimeCatalog into the process-global current_runtime via setRuntime(cat); setRuntime only frees the *previous* catalog on the next project transition. Shutdown is not a transition, so the last-loaded catalog's arena -- holding the entries/pin_styles slices grown from ArrayLists in loadFromPath -- outlived App.deinit and was reported as a leaked ArrayList grow by the DebugAllocator on window close. Fix: App.deinit now calls flow_node_catalog.setRuntime(null), reverting to the static fallback and freeing the active runtime catalog. Adds a leak-checked regression test that loads a real sidecar, installs it as the active catalog (the project-load path), and reverts exactly once (the shutdown path). Verified the test reproduces the leak: with the revert skipped it reports 'Memory Leak Detected' under testing.allocator. zig build clean; zig build test 464/464; zig build gui-test 26/26. --- src/app.zig | 9 ++++++ src/flow_node_catalog.zig | 59 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/src/app.zig b/src/app.zig index 7f9028b..2f3671d 100644 --- a/src/app.zig +++ b/src/app.zig @@ -432,6 +432,15 @@ pub const App = struct { // Flow Runtime frees its subscribed-flow name copies. self.entity_inspector.deinit(); self.flow_runtime.deinit(self.allocator); + // Revert the process-global flow-node catalog to its static + // fallback, freeing the project's runtime catalog (and its arena) + // if one was installed by `reloadFlowNodeCatalog`. Without this, + // the last-loaded `RuntimeCatalog` outlives the App: `setRuntime` + // only frees the *previous* catalog on the next project + // transition, and shutdown is not a transition — so the catalog's + // arena (the materialized `entries`/`pin_styles` slices grown from + // ArrayLists in `loadFromPath`) leaks on exit (labelle-gui#209). + flow_node_catalog.setRuntime(null); self.project_manager.deinit(); self.tree_view.deinit(); self.compiler.deinit(); diff --git a/src/flow_node_catalog.zig b/src/flow_node_catalog.zig index 557f435..3c31713 100644 --- a/src/flow_node_catalog.zig +++ b/src/flow_node_catalog.zig @@ -1148,6 +1148,65 @@ test "setRuntime: swap to runtime, then revert to static" { try std.testing.expectEqual(static_count, entries.len); } +// Regression for labelle-gui#209: a runtime catalog installed by a +// project load (and never followed by another project transition) must +// be reclaimed at App shutdown. `App.deinit` now does this via a final +// `setRuntime(null)`. This test mirrors that lifecycle under +// `testing.allocator` — load a real sidecar, install it, then revert +// exactly once (as shutdown does). If the final revert is dropped, the +// catalog's arena (holding the ArrayList-grown `entries`/`pin_styles` +// slices) leaks and `testing.allocator` fails the test — which is the +// exact DebugAllocator leak the issue reported on window close. +test "setRuntime: shutdown revert frees a sidecar-loaded catalog (issue #209)" { + const aa = std.testing.allocator; + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + + const json = + \\{ + \\ "generated_at": "2026-06-09T00:00:00Z", + \\ "plugins": [ + \\ { + \\ "name": "synthetic", + \\ "flow_nodes": [ + \\ { + \\ "qualified": "synthetic.do_thing", + \\ "display_name": "Do Thing", + \\ "category": "synthetic", + \\ "docs": "", + \\ "kind": "command", + \\ "pins": [ + \\ { "name": "entity", "label": "Entity", "zig_type": "u32", "dir": "input", "default": null } + \\ ], + \\ "return_type": null + \\ } + \\ ], + \\ "pin_styles": [ + \\ { "zig_type": "MyStruct", "label": "My Struct", "color": [12, 34, 56] } + \\ ] + \\ } + \\ ] + \\} + \\ + ; + try tmp.dir.writeFile(io_global.io(), .{ .sub_path = "flow_catalog.json", .data = json }); + const path = try tmp.dir.realPathFileAlloc(io_global.io(), "flow_catalog.json", aa); + defer aa.free(path); + + // Project-load path: parse the sidecar and install it as the active + // runtime catalog (what `App.reloadFlowNodeCatalog` does). + const cat = try loadFromPath(aa, path); + setRuntime(cat); + try std.testing.expect(current_runtime != null); + try std.testing.expectEqualStrings("synthetic.do_thing", entries[0].name); + + // Shutdown path: the single revert `App.deinit` now performs. This + // is the *only* free of `cat`'s arena — drop it and the test leaks. + setRuntime(null); + try std.testing.expect(current_runtime == null); + try std.testing.expectEqual(static_pin_styles.len, pin_styles.len); +} + test "loadFromSidecar: returns null when no .labelle/* subdir has the file" { const aa = std.testing.allocator; var tmp = std.testing.tmpDir(.{}); From b2ee3cd9b626c94595cdcecbdc5648acc2243a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Tue, 9 Jun 2026 11:47:58 -0300 Subject: [PATCH 2/2] test: defer setRuntime(null) so a failed assertion doesn't leak (gemini #211) --- src/flow_node_catalog.zig | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/flow_node_catalog.zig b/src/flow_node_catalog.zig index 3c31713..bd3c54b 100644 --- a/src/flow_node_catalog.zig +++ b/src/flow_node_catalog.zig @@ -1197,6 +1197,11 @@ test "setRuntime: shutdown revert frees a sidecar-loaded catalog (issue #209)" { // runtime catalog (what `App.reloadFlowNodeCatalog` does). const cat = try loadFromPath(aa, path); setRuntime(cat); + // Guarantee cleanup even if an assertion below fails — otherwise an + // early exit leaks `cat` and leaves `current_runtime` dirty for the + // next test. `setRuntime` is idempotent, so this is a no-op after the + // explicit revert succeeds (gemini #211). + defer setRuntime(null); try std.testing.expect(current_runtime != null); try std.testing.expectEqualStrings("synthetic.do_thing", entries[0].name);