emilsjolander
commented
6 years ago Agreed. This is a known issue and we have plans on moving all this into the context / node object. Not something we are working on right now but I hope we can have a look at solving this within the next month. If you have time we would also really appreciate a PR.
rockerhieu
NickGerleman
Report
Couldn't find any duplicate for "thread sanitizer" or "global".
When launching an app that uses Yoga concurrently, even when there is no concurrent access to a particular set of YGNodeRefs, the Thread Sanitizer triggers several warnings due to unprotected concurrent writing and reading from global variables.
Issues and Steps to Reproduce
Build and run an app that uses Yoga concurrently. This can be done with the Texture framework (texturegroup.org), or just by dispatch_async'ing layouts to various independent YGNodeRef trees.
Expected Behavior
Independent YGNodeRef trees should be thread-safe to operate on, as long as the app-provided implementation of the MeasureFunc and BaselineFunc are themselves thread-safe.
Actual Behavior
When running under TSAN, several operations are immediately flagged as not thread-safe. For more background on TSAN: https://developer.apple.com/videos/play/wwdc2016/412/
Link to Code
1st one: gNodeInstanceCount (very common) 2nd one: gCurrentGenerationCount (very common) 3rd one: gDepth (a bit less common than the above two)
Note that the C++ increment operator is not atomic: https://stackoverflow.com/a/39396999
Options to fix
Using C++ atomics would be one improvement. However this would be insufficient to create transactional integrity, e.g. if an atomic value is read and then a condition statement entered, reading the atomic again to do an operation with it is not transactional.
The best fix would be to avoid maintaining global variables if at all possible, and instead store these on the root YGNodeRef. If locking is added, it should be done with care to ensure deadlocks and thread contention are not an issue.