Safe access to a `#[thread_local]` should be disallowed

[alexcrichton]

Today this code compiles just fine, but it probably shouldn't:

#![feature(thread_local)]
#[thread_local]
static FOO: uint = 3;

fn main() {
    let a = &FOO;
    spawn(proc() {
        println!("{}", a);
    });
}

I'm not nominating this because thread_local is behind a feature gate, just wanted an issue to track it.

[alexcrichton]

Another route would just be disallowing borrows as well, similar to how we only allow borrows of statics in statics.

[thestinger]

It could be allowed if it gave borrows something like a 'task lifetime instead of 'static.

[arielb1]

Where is the unsoundness there? uint implements Sync, so sending a reference to it to a subtask is completely fine (unless the inner task can outlive the outer task, in which case the proc shouldn't be :'static and the spawn should be illegal).

[alexcrichton]

The lifetime 'static means that it outlives the entire program, but the lifetime of a thread-local variable is just for the thread that it's running on, which doesn't necessarily outlive the entire program. This means that interior pointers can be sent to other threads which become stale once the sending thread exits.

[thestinger]

It seems like it would be really easy to add 'task and keep it behind the same feature gate to make this sound. Eventually, #[thread_local] will be available on all relevant platforms and it could be used directly to handle simple cases rather than using wrappers. It's missing C++11 style destructor support so it isn't able to completely eliminate the need for a higher-level API.

[thestinger]

@alexcrichton: It also permits data races now that internal mutability in a static is permitted.

[eddyb]

Do we need 'task? Couldn't we have a NoSend wrapper around &'static T, implementing Deref?

[thestinger]

@eddyb: It needs 'task to be sound as an unwrapped static variable. It's necessary to expose a proper &T reference regardless.

[arielb1]

@thestinger

Preventing data races is a job for Sync (rather than Send), which (for some weird reason) #[thread_local] seem to require.

@eddyb
Having values that are 'static but not really does not seem like a good idea to me – it's fighting against the lifetime system.

[thestinger]

Preventing data races is a job for Sync (rather than Send), which (for some weird reason) #[thread_local] seem to require.

A #[thread_local] static requiring Sync is a recently introduced bug. It's not a mitigating factor for this bug, which does allow data races in the absence of that other bug.

[pythonesque]

I don't think adding a new 'task lifetime is actually that helpful for thread-locals. If they are Send, and are using fork-join concurrency, 'task means something different in a child thread from a parent thread, so a child 'task might outlive a parent 'task--how do you disambiguate? Does Rust know when the new 'task lifetime is created in the child so it can reason about this soundly? If it does, and they have different lifetime names, why use 'task at all and not just a regular lifetime?

On the other hand, if you can't Send them, just make them NoSend and have done with it. The only problem with making thread locals NoSend is that then you can't send them between tasks in fork-join style APIs. If you can't resolve the issues above, there's probably no sane way of determining that anyway so you might as well ban it (and really, I think it's a pretty niche usecase to want shared thread local data).

I don't think semantics are a good argument here. There are lots of examples of things that aren't "semantically" 'static, like data in Rcs (which can never outlive its calling thread) that nonetheless get along without special lifetimes.

[thestinger]

The point of 'task is that it would be like 'static but would not be Send.

I don't think adding a new 'task lifetime is actually that helpful for thread-locals. If they are Send, and are using fork-join concurrency, 'task means something different in a child thread from a parent thread, so a child 'task might outlive a parent 'task--how do you disambiguate? Does Rust know when the new 'task lifetime is created in the child so it can reason about this soundly? If it does, and they have different lifetime names, why use 'task at all and not just a regular lifetime?

It would be trivial to add it, and it doesn't need to know anything about other tasks.

On the other hand, if you can't Send them, just make them NoSend and have done with it. The only problem with making thread locals NoSend is that then you can't send them between tasks in fork-join style APIs. If you can't resolve the issues above, there's probably no sane way of determining that anyway so you might as well ban it (and really, I think it's a pretty niche usecase to want shared thread local data).

It's possible to get a reference to the value inside the thread-local variable, and it should be considered as having a 'static lifetime but without being considered Send. It will be possible to do this regardless of how you wrap it. It's possible to restrict the lifetime with a context object (like RefCell) but that's overly strict.

I don't think semantics are a good argument here. There are lots of examples of things that aren't "semantically" 'static, like data in Rcs (which can never outlive its calling thread) that nonetheless get along without special lifetimes.

That's not the point here. The Rc type isn't a form of static variable that needs to be provided by the compiler.

[pythonesque]

There is already a way to identify 'statics that are not Send, NoSend. As far as I know, it has all the properties you're looking for. What is the purpose of adding something functionally identical? Am I missing some way in which it would or could conceivably work differently?