Skip to content
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

NodeData and MeasureFunc are not thread-safe #146

Closed
colepoirier opened this issue Jun 11, 2022 · 3 comments · Fixed by #157
Closed

NodeData and MeasureFunc are not thread-safe #146

colepoirier opened this issue Jun 11, 2022 · 3 comments · Fixed by #157
Labels
enhancement New feature or request

Comments

@colepoirier
Copy link
Collaborator

What problem does this solve or what need does it fill?

"I am implementing a UI layer in an async framework, and I am unable to pass Stretch around (including using it as a member on the "self" structure) in async code because MeasureFunc is not Send/Sync."

Boxed(Box<dyn Fn(Size<Number>) -> Size<f32>>),

What solution would you like?

"I believe that the change may be as simple as changing the type of Boxed's value to sys::Box<dyn Fn(Size) -> Size> + Send + Sync)"

"This, unfortunately, could be a breaking change since this is a publicly exposed enum, so before jumping in to create a PR, I wanted to see what the opinion would be of attempting this change. I'm new enough to understanding how async works that I am not sure what the likelihood that these functions in most codebase wouldn't already be Send + Sync."

What alternative(s) have you considered?

N/A

Additional context

Migrating from vislyhq/stretch#69.

Because this is a migration of an issue, I am reproducing the original using direct quotations (indicated using "..") from the original issue author @ecton but modifying the url to point to taffy code.

@alice-i-cecile
Copy link
Collaborator

Thanks! I think the easiest and best solution to this is actually just to remove MeasureFunc completely, per #101.

@colepoirier
Copy link
Collaborator Author

Thanks! I think the easiest and best solution to this is actually just to remove MeasureFunc completely, per #101.

Great! I guess the pr for that should reference this issue so it closes once it is merged?

@alice-i-cecile
Copy link
Collaborator

Yes, exactly.

@alice-i-cecile alice-i-cecile changed the title [Feature] Support async functions in rust NodeData and MeasureFunc are not thread-safe Jun 12, 2022
nicoburns added a commit to nicoburns/bevy that referenced this issue Apr 6, 2023
DioxusLabs/taffy#146 has now been merged so FlexSurface is now
automatically Send and Sync without the unsafe impls!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants