How can users of taffy work around a potential stack overflow?

[osiewicz]

Hi,
At Zed we're trying to eliminate some of the sources of potential stack overflow with our rendering code; we've identified one potential avenue for such overflow in taffy, as it does not seem to take any measures to avoid it.
We've built a toy example for a deep element tree and even with use of stacksafe we're still seeing crashes in this example that trace back to taffy (¹).
Such stack overflow can also be reproduced with super deep benchmark with larger tree sizes ².

Is there any prior art with taffy for dealing with these kinds of issues? These stack overflows are not a huge issue for us, but we do see these in the wild and so ideally we'd love to solve that somehow directly within taffy (an alternative is for us to fork taffy and apply stacksafe ourselves, but we'd rather refrain from doing that). Our GPUI library is also being used by external parties and so it feels kind of bad for us to have a potential crashes lurking in the wild - especially one we have little control over in practice.

Footnotes

1. taffy-stack-overflow-853.txt ↩

2. I went with 20k, but there's probably an earlier cutoff point available. ↩

[nicoburns]

Some thoughts:

• I'm surprised you've seen this in the wild (this is surprising enough to me that you might want to consider looking for a bug in the UI definition code). When I surveyed popular websites, the absolute deepest tree (facebook.com IIRC) I found had a depth of ~60, which I believe is well under what would trigger a stackoverflow. Typical websites were in the 10-30 range.
• Perhaps the best remedy here is just to ignore content at a greater depth than that? We could build that into Taffy, or you could simply not push such content into Taffy?
• If you used the low-level Taffy API then you could apply stacksafe without forking. If you are also storing the Taffy styles in a non-Taffy data structure then this would also likely be a nice memory usage win.
• stacksafe looks pretty trivial to apply: just a proc_macro in a couple of places, so I'd be happy to accept a PR that used it behind a feature flag.

[osiewicz]

I'm surprised you've seen this in the wild (this is surprising enough to me that you might want to consider looking for a bug in the UI definition code)

Yeah, it's a bit hard to repro for us. Still, we'd rather eliminate one avenue for crashes altogether. We're quite lenient when it comes to tracking the depth of our layout tree.

I'd be happy to accept a PR that used it behind a feature flag.

I'm happy to hear that! I'll work on that then, thanks!