diff --git a/packages/core/src/zig/tests/wrap-cache-perf_test.zig b/packages/core/src/zig/tests/wrap-cache-perf_test.zig index 7316289da..faae6c997 100644 --- a/packages/core/src/zig/tests/wrap-cache-perf_test.zig +++ b/packages/core/src/zig/tests/wrap-cache-perf_test.zig @@ -25,31 +25,42 @@ test "word wrap complexity - width changes are O(n)" { view.setWrapMode(.word); const widths = [_]u32{ 60, 70, 80, 90, 100 }; - var times: [widths.len]u64 = undefined; - - // Warmup - view.setWrapWidth(50); - _ = view.getVirtualLineCount(); - - // Measure first (uncached) call for each width - for (widths, 0..) |width, i| { - view.setWrapWidth(width); - var timer = std.time.Timer.start() catch unreachable; - _ = view.getVirtualLineCount(); - times[i] = timer.read(); + + // Run multiple iterations and use median to reduce noise from CI variability + const iterations = 5; + var median_times: [widths.len]u64 = undefined; + + for (widths, 0..) |width, width_idx| { + var iter_times: [iterations]u64 = undefined; + + for (0..iterations) |iter| { + // Reset cache by setting a different width first + view.setWrapWidth(50); + _ = view.getVirtualLineCount(); + + view.setWrapWidth(width); + var timer = std.time.Timer.start() catch unreachable; + _ = view.getVirtualLineCount(); + iter_times[iter] = timer.read(); + } + + // Sort and take median + std.mem.sort(u64, &iter_times, {}, std.sort.asc(u64)); + median_times[width_idx] = iter_times[iterations / 2]; } var min_time: u64 = std.math.maxInt(u64); var max_time: u64 = 0; - for (times) |t| { + for (median_times) |t| { min_time = @min(min_time, t); max_time = @max(max_time, t); } const ratio = @as(f64, @floatFromInt(max_time)) / @as(f64, @floatFromInt(min_time)); - // All times should be roughly similar since text size is constant - try std.testing.expect(ratio < 3.0); + // All times should be roughly similar since text size is constant. + // Use a generous threshold (5x) to account for CI runner variability. + try std.testing.expect(ratio < 5.0); } test "word wrap - virtual line count correctness" { diff --git a/packages/react/src/reconciler/renderer.ts b/packages/react/src/reconciler/renderer.ts index 145333415..e9bd29e9d 100644 --- a/packages/react/src/reconciler/renderer.ts +++ b/packages/react/src/reconciler/renderer.ts @@ -1,4 +1,4 @@ -import { CliRenderer, engine } from "@opentui/core" +import { CliRenderer, CliRenderEvents, engine } from "@opentui/core" import React, { type ReactNode } from "react" import type { OpaqueRoot } from "react-reconciler" import { AppContext } from "../components/app" @@ -29,6 +29,17 @@ export type Root = { export function createRoot(renderer: CliRenderer): Root { let container: OpaqueRoot | null = null + const cleanup = () => { + if (container) { + reconciler.updateContainer(null, container, null, () => {}) + // @ts-expect-error the types for `react-reconciler` are not up to date with the library. + reconciler.flushSyncWork() + container = null + } + } + + renderer.once(CliRenderEvents.DESTROY, cleanup) + return { render: (node: ReactNode) => { engine.attach(renderer) @@ -43,16 +54,7 @@ export function createRoot(renderer: CliRenderer): Root { ) }, - unmount: (): void => { - if (!container) { - return - } - - reconciler.updateContainer(null, container, null, () => {}) - // @ts-expect-error the types for `react-reconciler` are not up to date with the library. - reconciler.flushSyncWork() - container = null - }, + unmount: cleanup, } } diff --git a/packages/react/tests/destroy-crash.test.tsx b/packages/react/tests/destroy-crash.test.tsx new file mode 100644 index 000000000..063716879 --- /dev/null +++ b/packages/react/tests/destroy-crash.test.tsx @@ -0,0 +1,91 @@ +import { describe, expect, it } from "bun:test" +import React, { useEffect, useState } from "react" +import { createTestRenderer } from "@opentui/core/testing" +import { createRoot } from "../src/reconciler/renderer" + +/** + * Regression test for: Native Yoga crash when renderer.destroy() is called + * with pending React state updates. + * + * Bug report: https://gist.github.com/rauchg/b2e1e964f88773a5d08f5f682dce2224 + * + * Issue: When calling renderer.destroy() while async operations (intervals, + * promises, etc.) are still updating React state, Yoga crashes with: + * "Cannot add child: Nodes with measure functions cannot have children." + * followed by "RuntimeError: Out of bounds memory access" + * + * The crash happens because: + * 1. renderer.destroy() is called (WITHOUT unmounting React first) + * 2. This calls root.destroyRecursively() which calls yogaNode.free() on all nodes + * 3. The interval keeps firing setLines() because React wasn't unmounted + * 4. React tries to re-render, calling appendChild() which calls add() + * 5. add() calls yogaNode.insertChild() on a freed (parent) yoga node + * 6. Yoga WASM crashes with out-of-bounds memory access + * + * The "Cannot add child: Nodes with measure functions cannot have children" + * error message is misleading - it's actually a use-after-free crash where + * the freed yoga node's memory has been reused/corrupted. + * + * CRITICAL: The bug only occurs when React is NOT unmounted before/during destroy. + * In the bug report, there is NO root.unmount() call - just renderer.destroy(). + */ + +describe("Renderer Destroy Crash with Pending React Updates", () => { + it("should not crash when renderer is destroyed without unmounting React while interval updates state", async () => { + // This test reproduces the EXACT scenario from the bug report: + // - Component has an interval updating state + // - renderer.destroy() is called WITHOUT calling root.unmount() + // - The interval continues to fire setLines() AFTER destroy + // - React tries to re-render on destroyed Yoga nodes -> CRASH + + const testSetup = await createTestRenderer({ + width: 40, + height: 20, + // CRITICAL: NO onDestroy callback - React is NOT unmounted + // This is exactly what happens in the bug report + }) + + const root = createRoot(testSetup.renderer) + + function App() { + const [lines, setLines] = useState([]) + + useEffect(() => { + // Interval keeps firing after destroy() because React isn't unmounted + const interval = setInterval(() => { + setLines((prev) => [...prev, `Line ${prev.length + 1}`]) + }, 5) + + return () => clearInterval(interval) + }, []) + + return ( + + OpenTUI Crash Repro + {lines.slice(-10).map((line, i) => ( + {line} + ))} + + ) + } + + root.render() + + // Let the component mount and interval start + await testSetup.renderOnce() + await Bun.sleep(30) + await testSetup.renderOnce() + + // Destroy WITHOUT unmounting React - this is the bug! + // The interval will keep firing setLines() after this + // React will try to add new elements to destroyed Yoga nodes + testSetup.renderer.destroy() + + // Wait for interval to fire more updates after destroy + // This is when the crash occurs if the bug is present + await Bun.sleep(100) + + // If we reach here without crashing, the bug is fixed + expect(true).toBe(true) + }) +})