-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
0 layoutWidth and 0 layoutHeight after repeated layout calculation #1678
Comments
Update: On first sight this didn't make sense since these values aren't set anywhere. So the issue can be avoided by making sure the Java config object is not garbage collected:
Or in other words: The caller of the library must take care of keeping the Java config alive, since the Yoga library is not taking care of it. Kudos to @rtPag for helping to find the issue! |
One option to make it safer for everyone using the public APIs would be to keep a reference to the config as a private member variable in the |
Huge thank you @michaeltroger @rtPag 🎉
This seems like the correct solution to me, and is consistent with how the JNI bindings handle Yoga node garbage collection. I think the production usages we have of JNI bindings all happened to use global duration configs, which is why we hadn’t seen it. |
Summary: Fixes [https://github.com/facebook/yoga/issues/1678](https://github.com/facebook/yoga/issues/1678) As described in the linked Issue, the problem is that the `YogaConfig` can get garbage collected by the JVM, while a `YogaNode` is still referring to it. This at some point leads to unexpected behaviour (0 values for `layoutWidth`/`layoutHeight`). The change coming with this PR makes sure the `YogaConfig` can not get garbage collected while it's used by a `YogaNode`. Demo project to confirm the fix https://github.com/michaeltroger/yogabug Kudos to rtPag, who helped identifying the issue. X-link: facebook/yoga#1703 Reviewed By: mdvacca Differential Revision: D63416127 Pulled By: NickGerleman
Summary: X-link: facebook/react-native#46651 Fixes [https://github.com/facebook/yoga/issues/1678](https://github.com/facebook/yoga/issues/1678) As described in the linked Issue, the problem is that the `YogaConfig` can get garbage collected by the JVM, while a `YogaNode` is still referring to it. This at some point leads to unexpected behaviour (0 values for `layoutWidth`/`layoutHeight`). The change coming with this PR makes sure the `YogaConfig` can not get garbage collected while it's used by a `YogaNode`. Demo project to confirm the fix https://github.com/michaeltroger/yogabug Kudos to rtPag, who helped identifying the issue. Pull Request resolved: #1703 Reviewed By: mdvacca Differential Revision: D63416127 Pulled By: NickGerleman fbshipit-source-id: efd87dac897e44d3664c228c40cda90f1e11c4f6
Summary: X-link: facebook/react-native#46651 Fixes [https://github.com/facebook/yoga/issues/1678](https://github.com/facebook/yoga/issues/1678) As described in the linked Issue, the problem is that the `YogaConfig` can get garbage collected by the JVM, while a `YogaNode` is still referring to it. This at some point leads to unexpected behaviour (0 values for `layoutWidth`/`layoutHeight`). The change coming with this PR makes sure the `YogaConfig` can not get garbage collected while it's used by a `YogaNode`. Demo project to confirm the fix https://github.com/michaeltroger/yogabug Kudos to rtPag, who helped identifying the issue. X-link: facebook/yoga#1703 Reviewed By: mdvacca Differential Revision: D63416127 Pulled By: NickGerleman fbshipit-source-id: efd87dac897e44d3664c228c40cda90f1e11c4f6
Summary: Pull Request resolved: #46651 Fixes [https://github.com/facebook/yoga/issues/1678](https://github.com/facebook/yoga/issues/1678) As described in the linked Issue, the problem is that the `YogaConfig` can get garbage collected by the JVM, while a `YogaNode` is still referring to it. This at some point leads to unexpected behaviour (0 values for `layoutWidth`/`layoutHeight`). The change coming with this PR makes sure the `YogaConfig` can not get garbage collected while it's used by a `YogaNode`. Demo project to confirm the fix https://github.com/michaeltroger/yogabug Kudos to rtPag, who helped identifying the issue. X-link: facebook/yoga#1703 Reviewed By: mdvacca Differential Revision: D63416127 Pulled By: NickGerleman fbshipit-source-id: efd87dac897e44d3664c228c40cda90f1e11c4f6
Should be fixed by the attached commit |
Report
Issues and Steps to Reproduce
General setup:
Actual Yoga API usage making it reproducible:
The issue happens already with a single YogaNode and setting only width/height (see also sample app), but the issue is not limited to this scenario. I have reasons to believe that it happens more often the bigger the tree is, but I thought a single YogaNode might make debugging the issue easier. If the tree is bigger then individual Nodes are affected but not necessarily all Nodes of the tree.
Expected Behavior
I would expect that consecutive calls with the same input always result in the same output.
Actual Behavior
After some iterations (repeating the resetting of the YogaNode, the applying of properties and the calculation of the layout),
layoutWidth
andlayoutHeight
deliver 0 values.It doesn't happen after a fixed amount of repetitions. It's mostly influenced through the delay between the calls and also the context in which the code is executed matters.
E.g. I had the issue in the provided sample app after:
One needs to play around with these delays though. In a company internal Demo app it needs only 5 iterations with a delay of 5 seconds in between, while I see no issue with such delay in the provided sample app at all.
Coming to the actual production impact: I originally found the issue when rendering a layout with a root and 2 children boxes, each of them having another child box. After re-calculating the layout ~5 times, some boxes started to disappear and the debugging led me to the 0 values.
We were using Yoga 1.19.0 until now and the same code in Yoga 1 returns approximately after the same iterations, floating point numbers that are not entirely correct, but if rounded you wouldn't see any issue.
For a size of 200x200, it returns 199.99997x199.99997 after 350 iterations in the sample app.
The issue with 0 values happens from Yoga 2.0.0 and still in 3.1.0.
Link to Code
A minimal reproducible Android app can be found here:
https://github.com/michaeltroger/yogabug
The app purposely crashes when the calculation failed. The crash can then be checked in the logs e.g.:
java.lang.IllegalStateException: unexpected size: 0.0x0.0 in iteration 316
The most relevant code snippet (Kotlin language):
see also:
https://github.com/michaeltroger/yogabug/blob/main/app/src/main/java/com/michaeltroger/yogabug/MainActivity.kt
The text was updated successfully, but these errors were encountered: