Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions packages/core/src/zig/tests/wrap-cache-perf_test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
24 changes: 13 additions & 11 deletions packages/react/src/reconciler/renderer.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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,
}
}

Expand Down
91 changes: 91 additions & 0 deletions packages/react/tests/destroy-crash.test.tsx
Original file line number Diff line number Diff line change
@@ -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<string[]>([])

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 (
<box flexDirection="column" border borderStyle="single">
<text bold>OpenTUI Crash Repro</text>
{lines.slice(-10).map((line, i) => (
<text key={`line-${i}-${line}`}>{line}</text>
))}
</box>
)
}

root.render(<App />)

// 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 <text> 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)
})
})
Loading