fix: clear staticNode reference on Static unmount#905
Conversation
|
CI is failing |
When a <Static> component unmounts, rootNode.staticNode was not cleared, leaving a dangling reference to a freed Yoga WASM node. Subsequent renders would call getComputedWidth/Height on the freed node, returning garbage values that caused OOM when allocating the output grid. Clear the staticNode reference in both removeChild and removeChildFromContainer when an internal_static node is removed. Fixes vadimdemedes#904
Switch boolean short-circuit to ternary for jsx-no-leaked-render and extract any-typed value before chaining .includes() to satisfy no-unsafe-call.
86fd9e0 to
d8f7516
Compare
|
The new regression test does not actually prove #904 is fixed. It passes on current |
Replace the simple unmount-and-check test with one that: - Rerenders multiple times after unmounting Static - Verifies output length stays stable (detects if stale staticNode causes fullStaticOutput to grow) - References issue vadimdemedes#904 in the test name The underlying use-after-free is nondeterministic (depends on WASM memory reuse timing), so this test functions as a smoke test rather than a guaranteed reproduction.
|
You're right, the previous test passed on master without the patch. I've updated it to verify that To be transparent: the underlying use-after-free is nondeterministic (depends on WASM allocator reusing the freed node's memory), so a fully deterministic unit test isn't possible without exposing reconciler internals. This test serves as a smoke test. If you'd prefer a more direct assertion, I could export a test helper from the reconciler to verify |
Summary
When a
<Static>component unmounts,rootNode.staticNodewas not cleared, leaving a dangling reference to a freed Yoga WASM node. Subsequent renders calledgetComputedWidth()/getComputedHeight()on the freed node, returning garbage values that caused OOM when allocating the output grid.This clears the
staticNodereference in bothremoveChildandremoveChildFromContainerwhen aninternal_staticnode is removed.Fixes #904
Test plan
<Static>, unmounts it via rerender, and verifies subsequent renders succeed without crashing