[PR #431] [MERGED] fix(core): fix memory leaks in Zig unicode encoding and grapheme pool #1312

New issue

Closed

opened 2026-03-14 09:30:06 +03:00 by kerem · 0 comments

kerem commented

2026-03-14 09:30:06 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/anomalyco/opentui/pull/431
Author: @edlsh
Created: 12/19/2025
Status: ✅ Merged
Merged: 12/22/2025
Merged by: @kommander

Base: main ← Head: fix/zig-memory-leaks

📝 Commits (6)

a000c77 fix(core): fix memory leaks in Zig unicode encoding and grapheme pool
f2c6782 test(core): add regression tests for memory leak fixes
294ed80 fix(test): use const for unmutated success variable
530c1a9 chore: remove build artifact
7e57a7e test: verify slot cleanup via WrongGeneration check
232bc19 fix(core): memory leaks, race conditions, and additional tests

📊 Changes

10 files changed (+0 additions, -0 deletions)

View changed files

📝 packages/core/src/lib/tree-sitter/client.test.ts (+15 -0)
📝 packages/core/src/lib/tree-sitter/client.ts (+2 -0)
📝 packages/core/src/zig/buffer.zig (+4 -6)
📝 packages/core/src/zig/grapheme.zig (+32 -1)
📝 packages/core/src/zig/lib.zig (+30 -0)
📝 packages/core/src/zig/renderer.zig (+13 -0)
📝 packages/core/src/zig/test.zig (+2 -0)
📝 packages/core/src/zig/tests/buffer_test.zig (+20 -0)
➕ packages/core/src/zig/tests/memory_leak_regression_test.zig (+159 -0)
📝 packages/core/src/zig/text-buffer-view.zig (+3 -0)

📄 Description

Summary

Fixes critical memory leaks in the Zig native code for unicode encoding and grapheme pool operations.

Changes

1. Fix `errdefer` that never runs in `encodeUnicode` (lib.zig)

Problem: The function returns bool, but errdefer only triggers on error union returns. All failure paths used catch return false, which is a normal return—not an error return—so cleanup never executed.

Fix: Replace errdefer with defer + success flag pattern:

Added var success = false; at start
Changed errdefer to defer { if (!success) { ... } }
Set success = true; before final return true;

2. Fix grapheme leak on realloc failure (lib.zig)

Problem: After pool.alloc() + pool.incref(), if realloc fails, the newly allocated grapheme wasn't tracked yet in the result array, so it would leak.

Fix: Track pending grapheme with var pending_gid: ?u32 = null;:

Set pending_gid = gid immediately after allocation
Clear pending_gid = null after successfully stored in result array
Cleanup decrefs pending_gid if set

3. Add bounds checks in GraphemePool (grapheme.zig)

Problem: incref, decref, get, and getRefcount didn't validate class_id before array access.

Fix: Added if (class_id >= MAX_CLASSES) return GraphemePoolError.InvalidId; to all four methods.

Testing

bun run build passes (all 6 platform binaries)
bun run test:native passes (1241/1242 tests, 1 skipped)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/anomalyco/opentui/pull/431 **Author:** [@edlsh](https://github.com/edlsh) **Created:** 12/19/2025 **Status:** ✅ Merged **Merged:** 12/22/2025 **Merged by:** [@kommander](https://github.com/kommander) **Base:** `main` ← **Head:** `fix/zig-memory-leaks` --- ### 📝 Commits (6) - [`a000c77`](https://github.com/anomalyco/opentui/commit/a000c77ca24715d5cc85d5720910e471abc159c4) fix(core): fix memory leaks in Zig unicode encoding and grapheme pool - [`f2c6782`](https://github.com/anomalyco/opentui/commit/f2c6782a8a1c5a90e39915fb2e4ec78a685da1b0) test(core): add regression tests for memory leak fixes - [`294ed80`](https://github.com/anomalyco/opentui/commit/294ed808a0670d1772f189e7423137a5b6605b1a) fix(test): use const for unmutated success variable - [`530c1a9`](https://github.com/anomalyco/opentui/commit/530c1a920de476058ef5f0d3213bd7ab44d4469a) chore: remove build artifact - [`7e57a7e`](https://github.com/anomalyco/opentui/commit/7e57a7eec6607d47a8383d65e50c74d10b9e1743) test: verify slot cleanup via WrongGeneration check - [`232bc19`](https://github.com/anomalyco/opentui/commit/232bc19e87323559b812e14134afb7b5e3adfb7a) fix(core): memory leaks, race conditions, and additional tests ### 📊 Changes **10 files changed** (+0 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `packages/core/src/lib/tree-sitter/client.test.ts` (+15 -0) 📝 `packages/core/src/lib/tree-sitter/client.ts` (+2 -0) 📝 `packages/core/src/zig/buffer.zig` (+4 -6) 📝 `packages/core/src/zig/grapheme.zig` (+32 -1) 📝 `packages/core/src/zig/lib.zig` (+30 -0) 📝 `packages/core/src/zig/renderer.zig` (+13 -0) 📝 `packages/core/src/zig/test.zig` (+2 -0) 📝 `packages/core/src/zig/tests/buffer_test.zig` (+20 -0) ➕ `packages/core/src/zig/tests/memory_leak_regression_test.zig` (+159 -0) 📝 `packages/core/src/zig/text-buffer-view.zig` (+3 -0) </details> ### 📄 Description ## Summary Fixes critical memory leaks in the Zig native code for unicode encoding and grapheme pool operations. ## Changes ### 1. Fix `errdefer` that never runs in `encodeUnicode` (lib.zig) **Problem**: The function returns `bool`, but `errdefer` only triggers on error union returns. All failure paths used `catch return false`, which is a normal return—not an error return—so cleanup never executed. **Fix**: Replace `errdefer` with `defer` + success flag pattern: - Added `var success = false;` at start - Changed `errdefer` to `defer { if (!success) { ... } }` - Set `success = true;` before final `return true;` ### 2. Fix grapheme leak on realloc failure (lib.zig) **Problem**: After `pool.alloc()` + `pool.incref()`, if `realloc` fails, the newly allocated grapheme wasn't tracked yet in the result array, so it would leak. **Fix**: Track pending grapheme with `var pending_gid: ?u32 = null;`: - Set `pending_gid = gid` immediately after allocation - Clear `pending_gid = null` after successfully stored in result array - Cleanup decrefs `pending_gid` if set ### 3. Add bounds checks in GraphemePool (grapheme.zig) **Problem**: `incref`, `decref`, `get`, and `getRefcount` didn't validate `class_id` before array access. **Fix**: Added `if (class_id >= MAX_CLASSES) return GraphemePoolError.InvalidId;` to all four methods. ## Testing - [x] `bun run build` passes (all 6 platform binaries) - [x] `bun run test:native` passes (1241/1242 tests, 1 skipped) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>