Tear down child nodes iteratively to avoid deep-tree stack overflow#396
Merged
aehlke merged 1 commit intoMay 15, 2026
Merged
Conversation
Node.childNodes holds strong references, so the default ARC release chain (Node.deinit -> release childNodes -> child Node.deinit -> ...) recurses once per nesting level. Deeply nested HTML overflows small thread stacks (Swift cooperative-pool workers, GCD workers, explicit Threads), crashing with EXC_BAD_ACCESS on Document teardown or Element.empty(). Add a Node.deinit that unwinds the subtree iteratively: children are drained onto a work stack, and a popped node held only by the stack (isKnownUniquelyReferenced) has its own children hoisted and its childNodes cleared before it is dropped, so it deinits with an empty childNodes and the chain never nests. Nodes still referenced elsewhere are left fully intact, making this behaviourally identical to the default recursive release. Adds StackOverflow393Test covering deep teardown and Element.empty() on a small-stack thread, plus shallow and main-thread controls. Closes scinfu#393
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.
Closes #393.
Problem
Node.childNodesis a strong-referenceArray<Node>. When a node's refcount reaches zero, the default ARC release chain is:That recurses once per HTML nesting level. Real-world pages routinely exceed 10k levels of nesting, which overflows the smaller stacks of non-main threads (Swift cooperative-pool workers, GCD workers, explicit
Threads). The result is anEXC_BAD_ACCESScrash onDocumentteardown orElement.empty()when the work happens off the main thread.Element.empty()hits the same recursive release through itschildNodes.removeAll().Fix
Add a
Node.deinitthat unwinds the subtree iteratively instead of recursively:isKnownUniquelyReferenced) is about to deinit anyway, so its own children are hoisted onto the stack and itschildNodescleared before it is dropped. It then deinits with an already-emptychildNodes, so the chain never nests.Elements, selector caches, or held by the caller) are left fully intact.This keeps stack depth bounded regardless of nesting depth, and is behaviourally identical to the default recursive release - it only changes the order of unwinding, not which nodes get freed.
Element.empty()needs no change of its own: once thedeinithook is in place, the children it releases tear down iteratively too.Testing
StackOverflow393Testruns teardown on aThreadwith a deliberately small 512 KB stack:testShallowOnSmallStackSurvives- control: shallow tree on a small stack.testDeepOnMainThreadSurvives- control: deep tree (12k levels) on the main thread's large stack.testDeepTeardownOnSmallStackSurvives- regression: deep tree released on a small-stack thread. Crashed withEXC_BAD_ACCESS/ SIGBUS before the fix.testDeepEmptyOnSmallStackSurvives- regression: same shape viaElement.empty()on a uniquely-held deep subtree.