Could non-capturing Fn-closures be Sync?


#1

Summary

During the discussion of the Custom Panic Handler RFC, the idea came up if we could Sync (some) closures. So to start the discussion, I’m writing this Pre-RFC.

Motivation

In multithreaded environments, it may be useful to share callbacks between threads. And callbacks are very commonly defined as closures. The following program tries to share a very simple closure between threads (that don’t even run concurrently):

use std::thread;
use std::sync::Arc;

fn main() {
    let x : &'static Fn(u32) = |l| { /* do something with l */ };
    let y = &Arc::new(x); // NOPE, this won't compile
    for v in 1..4 {
        println!("{:?}", thread::spawn(move|| {
           y(v);
        }).join())
    }
}

This will fail, because the trait core::marker::Sync is not implemented for the type core::ops::Fn(u32) [E0277].

Now, there is nothing within x that would preclude it from being Sync, but rustc doesn’t know that, so it balks.

Detailed Design

The compiler should already know whether a closure is immutable (because of the Fn declaration), and since it already keeps track of captured variables (so it can size the closure-struct), it should be simple enough to determine if a closure is eligible for Sync during compilation.

On detecting such closures, the compiler would then implicitly add a Sync trait bound. The error message for missing Sync on closures should be extended to suggest making the closure an Fn and/or show the captured state.

Drawbacks

  • It may not be obvious from the code if a closure captures any state. A good error message should remediate this.
  • It will make the compiler slower, because it has to do the aforementioned additional check for each closure definition.

Alternatives

  • Leave things as they are
  • Require the user to state their intention by adding Sync as a trait bound manually when defining the closure. This would reduce the burden on the compiler (since the check would only run if a closure is marked as Sync

Unresolved questions

All of them. This is a Pre-RFC, remember?


#2

Shouldn’t type inference bubble this up all the way to the closure definition so the compiler knows whether Sync is expected and then only do the check? Type annotations might be necessary in some locations.

On that note: Your code should actually include the Sync bound in the type definition of x.

let x : &'static (Fn(u32) + Sync) = |l| { /* do something with l */ };

Also your code won’t compile since you cannot pass a scope-local reference to a spawn thread. For now (and just this simple proof of concept code) the easiest would be to use scoped.

<anon>:10:14: 10:25 error: borrowed value does not live long enough
<anon>:10     let y = &Arc::new(x); // NOPE, this won't compile
                       ^~~~~~~~~~~
note: reference must be valid for the static lifetime...
<anon>:10:26: 16:2 note: ...but borrowed value is only valid for the block suffix following statement 1 at 10:25
<anon>:10     let y = &Arc::new(x); // NOPE, this won't compile
<anon>:11     for v in 1..4 {
<anon>:12         println!("{:?}", thread::spawn(move|| {
<anon>:13            y(v);
<anon>:14         }).join())
<anon>:15     }
          ...

Curiously enough, this is a fatal error, even though the compiler could easily continue… http://is.gd/B9t37L you can do the type checking by replacing the closure definition with unimplemented!()


#3

Absolutely.

In the meantime, whataloadofwhat on reddit suggested a working alternative:

#![feature(scoped)]

use std::{sync, thread};

fn main() {
    let a = 1;
    let x = |l: u32| println!("{} {}", a, l);
    let y = sync::Arc::new(&x as &(Fn(u32) + Sync));
    for v in 1..4 {
        let y = y.clone();
        println!("{:?}", thread::scoped(move|| {
           y(v);
        }).join());
    }
}

The great thing about this is that we can even capture (obviously immutably) upvalues, the only restriction is that our closure is not self-modifying (however it requires an Arc, which would not be needed were the Pre-RFC adopted).


#4

uhm… the following also works… no Arc, no as

#![feature(scoped)]

use std::thread;

fn main() {
    let x: &(Fn(u32) + Sync) = &|l| println!("{}", l);
    for v in 1..4 {
        println!("{:?}", thread::scoped(move|| {
           x(v);
        }).join());
    }
}

for spawn we need the 'static bound of course…

So your issue is actually not with Sync but with getting a &'static to a closure.


#5

So how do we get a &'static to a closure? Edit: If I change the let x: & in your last snippet to const x: &'static, I get an internal compiler error:

error: internal compiler error: no type for node 20: local l (id=20) in fcx 0x7feff0feef78
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: run with `RUST_BACKTRACE=1` for a backtrace
thread 'rustc' panicked at 'Box<Any>', /home/rustbuild/src/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libsyntax/diagnostic.rs:209

Error code 101

(On playpen) Bug report submitted: #25180