Repository navigation
Tracking Issue for scoped threads #93203
Description
Activity
- addedA-runtimeArea: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflowsArea: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflowsT-libs-api[DEPRECATED; DO NOT USE][DEPRECATED; DO NOT USE]C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCCategory: An issue tracking the progress of sth. like the implementation of an RFC
on Jan 22, 2022 Nominating this for @rust-lang/lang team discussion for the unresolved question.
- addedI-lang-nominatedNominated for discussion during a lang team meeting.Nominated for discussion during a lang team meeting.
on Jan 22, 2022 danielhenrymantilla commented
on Jan 22, 2022 ContributorMore actionsAwesome API, thanks again @m-ou-se for taking care of driving this 👍
To expand on my captures suggestion / "thingy"
I think it would generally make sense to be able to mark (
Copy) type definitions with a#[favor_capture_by_move]annotation on them or something, so that, in a non-generic context, if one haslet x = instance_of_that type; || uses(x) || uses(&x),xis nonetheless captured by move.
It sounds like a kind of straight-forward thing to implement? I'd be willing to try to tackle the implementation 🙂.
With it, we'd then replace&'scope Scope<'env>withScopeRef<'scope, 'env>(probably still namedScope, but I'm using theRefsuffix here to clarify the comparison with the current implementation). The reason for that would be to have that first'scopeparameter be non-covariant (≠ the (co)variance of'scopein&'scope Scope<'env>), so that the requirement for the scoped joined thread closure would becomeF : 'scoperather thanF : 'env, so thatFwould be allowed to capture theScopeRef.With all that, the API would become:
thread::scope(|scope| { … scope.spawn(|| { … scope.spawn(|| { … }); }); … });
which is not only neater (there is one scoped / scope-defining API, at the outermost closure layer, and then good old
.spawn(|| …)functions), but would also avoid the rather hard to grasp error messages that somebody gets when they update their scoped threads usage with a new nested thread.Indeed, consider somebody updating some code and doing:
fn main () { ::crossbeam::thread::scope(|scope| { scope.spawn(|_| { // … }); scope.spawn(|_| { + scope.spawn(|_| { /* … */ }) // … }); }) .expect("Some thread panicked"); }They'd get, as of the current
nightly, the following error message:error[E0521]: borrowed data escapes outside of closure --> src/main.rs:7:9 | 3 | ::crossbeam::thread::scope(|scope| { | ----- | | | `scope` declared here, outside of the closure body | `scope` is a reference that is only valid in the closure body ... 7 | / scope.spawn(|_| { 8 | | scope.spawn(|_| { /* … */ }) // <- ADDED! 9 | | // … 10 | | }); | |__________^ `scope` escapes the closure body here | = note: requirement occurs because of the type Scope<'_>, which makes the generic argument '_ invariant = note: the struct Scope<'env> is invariant over the parameter 'env = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
Now, I personally do love the quality of this error message, but I think we have to admit it can still be quite too "technical / advanced" for some Rust programmers out there (Alan yadda yadda): they shouldn't have to know about variance to know how to use a scoped API. Granted, another option would be to invest some effort in improving the error message when using a scoped API / hard-coding this very situation, but I'd rather see that effort invested on
#[favor_capture_by_move], for instance.
These have been my two cents on the topic, thanks for reading 🙏, and won't insist anymore on this subject, I promise 😄
Reacted by Mads Marquart, runiq, Ibraheem Ahmed, Harry Sarson, Ludwig Stecher and MojaHmm, regarding the capture of references, I think that the RFC 2229 logic might be relevant here. i.e. we could capture
*scopeby reference instead of capturingscopeby reference. In a way, I'm always surprised we don't.In the @rust-lang/lang meeting last week, I agreed to do a writeup about the question of whether a more ergonomic API is possible. I see that @danielhenrymantilla has a nice write-up here as well.
What was the question?
Is there any way to make it so that
spawncalls don't need to get a scope? In other words, instead of this:scope(|s| { s.spawn(|s| { s.spawn(|s| { ... }); }) })
can we have this?
scope(|s| { s.spawn(|| { // ^^ look ma, no `s` s.spawn(|| { ... }); // ^^ look ma, no `s` }) })
What is the answer?
No.
No? There's no way?
OK, fine: maybe someday. But not for a while.
Can't you be more specific?
If you insist, but remember that you asked for it.
I spent a while deep-diving here because the situation wasn't quite what I expected. To explain, though, I think we gotta go back to basics.
Question
Similar to Rayon and others, the current scoped API gives each task a reference to the scope that contains the task:
scope(|s| { // s here has type `&'a Scope` for some fresh lifetime 'a that is confined to this closure let s0 = s; s.spawn(|s| { // here, we shadow `s`; you get back a `&'b Scope` for some fresh `'b` lifetime specific to *this* closure // in reality, though, this `s` is the same as the outer one -- i.e., same pointer: assert_eq!(s as usize, s0 as usize); }); });
The question is, why is this inner
snecessary? Couldn't we just dos.spawn(|| { .. })and captures?Why
|| { s }doesn't work, take 1If you try to setup an API like the one above, you will find that it doesn't work. The problem is that the inner
|| sclosure capturessfrom the outer frame by reference. This turns out to be a general thing: any time that you have reads of a copy type, a non-move closure like|| swill always capture it by reference. The reason is that non-move closures take the "least permissions" that they can: they prefer to capture by&ref, then by&mut ref, and finally to take ownership ("by value").The downside of capturing by reference in this case is that it means the lifetime of the inner closure is linked to the outer closure, and we need them to be independent. We want the outer closure to return and then the inner closure is still running in a parallel thread, so it can't have a link to the outer closure.
Shouldn't we just always capture copy types by value?
So why DON'T we always capture copy types by value? It's debatable if the current behavior is right, but it wasn't an accident. For fun, here is an internals thread where we discussed this very question in 2015.
The intuition for the current rules is that a
||(non-move) closure is "part of the enclosing stack frame". The problem is that this is not the behavior we want here, we actually want something that is detached from its immediate parent, but attached to the parent of the scope itself. We don't have a way to express this (we'll return to this theme).This is not something we can "just change", it would change the semantics of existing programs in various ways. Consider this example:
let mut i = 0; let c = || println!("{}", i); i += 1; c();
Today, this code fails to compile, because
cgrabs a shared reference toi, preventing the mutationi += 1. If you changecto amoveclosure, the code compiles and prints0(not1, because it now has its own independent copy ofi).I suspect there are examples of code that runs in both versions but does something different, though at the moment I'm not sure what that is (if
Cellwere copy, it'd be easy to create such programs, but it's not, partly for this reason).What if users use
moveclosures by hand?OK, so if by-reference closures aren't quite what we want (because we want something detached from the caller), why not have people use a move closure? i.e., why not write
s.spawn(move || ....)? That would work fine for copy types likes, but not so well for things that are not copy.The problem is that the intuition for a move closure is that it is detached from the surrounding stack entirely. But that's not what we want here. As we said, we want something that remains attached to the parent stack, but not the immediate parent. We want it to be attached to the scope's parent.
If you use a move closure, patterns like this don't work, and that's annoying:
let v = vec![]; scope(|s| { s.spawn(move || { for e in &v { } }); s.spawn(move || { for e in &v { } }); })
Here, the two move closures would both try to take ownership of
v, which means this code won't compile. We'd have to writelet v = &vbefore spawning for this to work.Wait, what about RFC 2229?
So, the above is all accurate for Rust 2018, but as of Rust 2021 the situation is actually somewhat different. In particular, we can now capture places and not just variables. Actually, in Rust 2021, it turns out that we don't capture
s-- we capture*s. You can see this if you use some internal rustc annotations. Analyzing this program gives output that includes:... note: Min Capture s[Deref] -> ImmBorrow --> src/main.rs:23:25 | 23 | s.spawn( | ^ ...The
s[Deref]means that the closure is borrowing&*s, and not&s. This is important because it means that we don't actually capture data from the intermediate stack frame, which is going to terminate. We capture theScopeobject directly from its owner, which is thescopefunction, which (simplified) looks like this:fn scope<'env>(c: impl FnOnce(&Scope<'env>) + 'env) { let s = Scope::new(); c(&s) }
So why don't things work?
The role of the
'envlifetimeThe problem is this
'envlifetime. It represents "the lifetime of the closure's captured data", and it can basically be anything, but it has to be larger than the call toscopeitself. Therefore, so long asscopedoesn't return, that data remains valid (which is what we want for this API). ThespawnAPI requires that each thread's closure outlives'env:impl<'env> Scope<'env> { pub fn spawn<'scope, F, T>(&'scope self, f: F) -> ScopedJoinHandle<'scope, T> where F: FnOnce(&Scope<'env>) -> T + Send + 'env, T: Send + 'env; // ^^^^ here }
Putting pieces together
So, let's put it together:
- We have a lifetime
'envthat lives "sometime longer then the call toscope()" - Threads can only captaure things that live for
'env - We have a
svariable that threads need to capture which is a local variable withinscope()
You see the problem? Threads cannot capture
sbecause it doesn't live for'env. We can't quite express what we need to express, which is something like "some lifetime that is internal to'scopebut longer than the call to this closure". It's possible we'll grow ways to say that or tweak the system in the future, but I don't think it'll be soon. =)Other possible language additions
I could imagine us trying to tweak the closure capture rules here, but I'm not sure exactly how. In some ideal-ish world we've have a way for the compiler to know "how detached" the closure ought to be from the parent stack frame (e.g., based the lifetime bound that the
spawnAPI requires).I've advocated for a more explicit set of capture clauses, too, but that doesn't really solve anything here, since you wouldn't want to use those on a common basis.
I think that the context-capabilities proposal that @tmandry recently blogged about is relevant here (I view the "scope" as a kind of capability), but that's obviously far out.
In any case, any change here will require a fair bit of research and coordination.
Reacted by Daniel Henry-Mantilla, Josh Triplett, Paa Kojo Samanpa, runiq, Slanterns, Moja, João Marcos and TylerReacted by Josh Triplett, Moja, João Marcos and Remoun MetyasReacted by Mara Bos, J. Frimmel, scottmcm, Jon Gjengset, runiq, Slanterns, Moja and Remoun Metyas- We have a lifetime
- removedI-lang-nominatedNominated for discussion during a lang team meeting.Nominated for discussion during a lang team meeting.
on Feb 15, 2022 danielhenrymantilla commented
on Feb 15, 2022 ContributorMore actionsThanks for the detailed write-up, @nikomatsakis 🙏
It turns our that that 2021-edition-captures tangent about a
&Tbeing captured by "value" rather than by ref (&&Ton older editions) is actually a very interesting escape hatch. It basically provides one case of my advocated#[favor_capture_by_move], and one case is all I needed:&'scope Scope<'scope, 'env>
scope parameter that would be captured "by value", and with the inner
'scope, made interiorly invariant, so as to be able to useF : 'scoperather thanF : 'env.Here is a Playground, a bit dirty, tweaked from the original "capture by move" idea
Now to decide if such a complex type is warranted 😅
- More seriously: if hidden behind a type alias, the API docs could just show
scope: ScopeRef<'scope, 'env>, so it's not that bad.
Reacted by Niko Matsakis, Mario Carneiro, QuineDot, Muhammad Mominul Huque and Frank Steffahn- More seriously: if hidden behind a type alias, the API docs could just show
@danielhenrymantilla Heh, thanks for proving my conclusion a bit hasty! I think indeed that this scheme can work, though whether the type is too complex is a good question.
To elaborate a bit around this point, if we had the ability to write
for<'a where 'a: 'b>(not a syntax I would recommend, but hopefully you get the idea) then effectively what we want to be able to say is something likefn scope<'env>(f: impl for<'scope where 'env: 'scope> FnOnce(&'scope Scope<'scope>)
i.e., each of the spawned closures must outlive this
'scope, but they don't quite know what'scopeis, only that it is outlived by'env. So if they can prove they outlive'env, that's good enough.You can indeed often simulate where clauses like this by leveraging implied bounds, e.g.,
struct Foo<'a, 'b> where 'a: 'b, which is kind of the trick here, but it does make the signature more complex of course (as you noted).Thanks @nikomatsakis and @danielhenrymantilla!
@danielhenrymantilla, your suggestion is interesting! The type is a bit complex, but users don't have to write it out as they just get it as argument to their closure. It's definitely worth considering.
However, I'd really like to have some understandable explanation of the lifetimes, which is already quite tricky with the version that's currently implemented. If a user asks how the lifetimes work here, I'd like to be able to explain it without causing more confusion. @nikomatsakis's
for<.. where ..>explanation helps a bit here.(Not sure if that type alias helps much, since the definition of such an alias is public and part of the stable api anyway.)
danielhenrymantilla commented
on Feb 16, 2022 ContributorMore actionsThe gist regarding the lifetimes involved here, would be:
-
for<'scope where 'env ⊇ 'scope> F : UsableAtLeastWithin<'scope>
⇔
F : for<'scope where 'env ⊇ 'scope> UsableAtLeastWithin<'scope>
⇔
F : UsableAtLeastWithin<'env>- Where I'm using the
trait UsableAtLeastWithin<'region> = 'region;alias for better readability, and'a ⊇ 'bnotation for'a : 'b.
So we can replace the
F : 'envbounds withF : 'scopebounds, provided'scopebe invariant1
That way we can have the closures capture the…Scope…itself. - Where I'm using the
-
We achieve invariance of
'scopeby having it inside theScopetype, since otherwise, with the&'scopepart alone,'scopewould be covariant (the region'scopecould be shrunk by users). -
We need to have a
&'scope Scope<'scope>rather than just aScopeRef<'scope>because of this capture-by-move thing, where&'scope Scope<'scope>is captured as such, whereas aScopeRef<'scope>would be captured by reference, yielding a&'short_lived ScopeRef<'scope>, which would not meet the: 'scoperequirement. -
To get a
for<'scope where 'env : 'scope>quantification in current Rust, we carry an implicit'env : 'scopebound inside theScopetype itself, so thatfor<'scope>quantifications that "see" theScope<'scope, 'env>type end up correctly constrained. Hence whyScopealso needs to be infected with'env2.
-
As to what does
'envrepresent, it's a caller-chosen upper-bound on this quantification, and the caller will get maximal flexibility by picking a minimal'env. In Rust, the minimal'lifetimefed to a generic function call necessarily spans beyond the return of that call. In this instance,thread::scope(…):thread::scope(…) // | // | 'env cannot end before this call, and has no reason to end later. thread::scope(|scope| { … }) --------- // 'env cannot end before this call, and has no reason to end later.These lines I've drawn thus represent the limit after which stuff borrowed by the threads may die / be dropped.
Footnotes
-
if we were to let the caller "shrink
'scope", then they'd end up withF : UsableWithin<'small_enough_region>, which would always be met, resulting in a useless bound ↩ -
Technically an extra closure parameter could be the one mentioning
'env: it doesn't necessarily have to be tied toScopeitself (e.g.,f : impl for<'scope> FnOnce(&'scope Scope<'scope>, [&'scope &'env (); 0])). But we can agree that it wouldn't improve the readability 😅. ↩
Reacted by zjp-CN-
I tried the current nightly API in some test code I was writing, and it worked well. Served the purpose I wanted it for, borrowing some local values, without any issues.
What's the next step here? We have an existence proof that it's possible to eliminate the scope parameter. It might be possible to do so in a simpler way, but we don't know that yet. Can we hide some of the internals and the multiple lifetimes from the public API to allow for future evolution, perhaps by keeping the Scope type unstable for longer than the scope function?
77 remaining items
- addedrelnotesMarks issues that should be documented in the release notes of the next release.Marks issues that should be documented in the release notes of the next release.
on Jun 11, 2022 Merged and stabilized in 1.63!
Reacted by Juniper Tyree, Jane Losare-Lusby, Darek, runiq, Frank Steffahn, Jon Gjengset, Timo, Alex Macleod, Leonardo Gibrowski Faé, Weihang Lo and 18 moreDo we have a test case for the “borrow function arg ref example” code? I’m asking because apparently there’s additional work on NLL happening, and we probably wouldn’t want that test case to break before scoped threads are released, or otherwise (in case of a full revert on NLL) reconsider whether or not scoped threads should be already stabilized without it regardless, or later.
In any case having a test case seems important, I couldn’t find any so far, but also I don’t exactly know everywhere I’d have to look for it.
Reacted by Mads MarquartReopening for now. Can be closed with a test case added.
- addedE-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
on Jun 15, 2022 - removedto-announceAnnounce this issue on triage meetingAnnounce this issue on triage meeting
on Jun 16, 2022 - added a commit that references this issue
on Jun 16, 2022 Err I guess I should have reopened the other issue. Whoops. Oh well.
- added a commit that references this issue
on Sep 15, 2022
Feature gate:
#![feature(scoped_threads)]This is a tracking issue for scoped threads.
RFC: https://rust-lang.github.io/rfcs/3151-scoped-threads.html
Example usage
Public API
Documentation: https://doc.rust-lang.org/nightly/std/thread/fn.scope.html
Steps / History
Unresolved Questions
&Scopeargument to the functions given to.spawn()? That is,scope.spawn(|| ..)rather thanscope.spawn(|_| ..).move || ..instead, but that's not great. Maybe the language could be subtly changed to capture references or certainCopytypes by value rather than by reference(-to-reference).'envand'scopelifetimes clearly without scaring people away.