-
Notifications
You must be signed in to change notification settings - Fork 502
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
Missing Send bound in rayon-futures 0.1 #697
Comments
Yes, I think you're right.
See also #679, though that's probably not going to be its final form. @nikomatsakis wants to open an RFC to go through the questions raised in that PR. Regardless, I think we will want something based on Are you already using |
Not enough to have a particularly useful opinion about the best design. It's more that I read the code apropos to something else (maybe even idle curiosity, more likely a question on Reddit) and realized I have the skills to port it. Whether I think I have the bug itself solved, but I couldn't figure out how to make a |
#698 is in rayon-futures 0.1.1. |
ScopeFutureExt::spawn_future
should also haveSend
bounds on the output types ofF
Sync and Send are unsafely derived for ScopeFuture and the comment asserts that the implementation of
S
is more thread-safe than it appears. However thisunsafe impl
has the side effect of not checking the other types which are members of ScopeFutureContents, includingI'm porting
rayon-futures
to use version 0.3 of thefutures
crates and found this bug by trying to automatically implement Sync and Send. I haven't yet looked into whether it is safe to assumeScopeHandle: Send
(it doesn't feel good - adding that trait bound inrayon-core
throws errors), but even assuming it's safe I think it would be better to make that assumption only for thescope: Option<S>
field and let the compiler check the rest.I can't promise that I'll have time to fix this within the next few days, so I'm reporting it for triage. But I'll make at least a simple fix my first priority with whatever time I do find.
The text was updated successfully, but these errors were encountered: