Conversation
An exception thrown by a measure function unwinds the WebAssembly frames of calculateLayout without restoring Emscripten's stack pointer, so every throw leaks stack in proportion to the layout depth. After enough throws the stack overwrites static data and later calls fail with "memory access out of bounds", usually in YGNodeNew. Save the stack pointer before calling into the layout and restore it if an exception propagates, then rethrow. stackSave and stackRestore are added to EXPORTED_RUNTIME_METHODS.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
If a measure function throws during
calculateLayout(), the exception unwinds the WebAssembly frames without restoring Emscripten's stack pointer (emscripten#21350), so every throw leaks stack in proportion to the layout depth. The stack is 64 KB and sits directly above static data. Once enough has leaked, later calls fail withRuntimeError: memory access out of bounds(usually inYGNodeNew), and the module can't recover without being reloaded. Freeing nodes infinallydoesn't prevent this, because the leak is on the stack, not the heap.On
mainbuilt from source, it takes 156 throws with a single measured node, and 18 when the measured node is 20 levels deep.Changes
calculateLayout()saves the stack pointer before calling into Wasm. If an exception comes back, it restores the pointer and rethrows, so callers still get the original error. The nodes whose layout was interrupted stay dirty and are measured again on the next pass.stackSaveandstackRestoreare added toEXPORTED_RUNTIME_METHODS.#2033 handles the same problem for dirtied functions.
Test Plan
New tests in
YGMeasureTest.test.ts:measure_func_exception_propagates_to_callerlayout_works_after_repeated_measure_func_exceptions: 1,000 throws from a node 20 levels deep, then a normal layout returns the expected size. Onmainit fails withmemory access out of bounds.node_is_measured_again_after_its_measure_func_threwChecks run with Node 20 and emsdk 4.0.23 on an Apple M2:
javascript/:yarn testpasses 38 suites and 576 tests (573 before this change).yarn lintandyarn tscare clean, andyarn packsucceeds.yarn benchmark, three runs each: 3.7–5.3 ms per case onmain, 3.2–5.3 ms with this change.yarn format-check-javascriptandyarn format-check-cpppass. I moved my local, untrackedjavascript/.emsdkaside first, because Prettier would otherwise check it. I didn't run the Kotlin and Python checks, which need a local JDK; this change doesn't touch those files.