-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tracking issue for thread::Builder::spawn_unchecked #55132
Comments
@oliver-giersch what do you think about changing imp::Thread::new(stack_size, Box::new(main)) to imp::Thread::new(stack_size, transmute::<Box<FnBox()>, Box<FnBox()>>(Box::new(main))) This removes the burden of dealing with lifetime fumbling from the native implementations to where the unsafety is actually introduced. |
I'm sorry but I fail to see what this would accomplish, could you explain further? |
Context: I'm running into issues implementing a new target in Previously, the implementation was approximately this: pub fn spawn<F, T>(self, f: F) -> io::Result<JoinHandle<T>> where
F: FnOnce() -> T, F: Send + 'static, T: Send + 'static
{
imp::Thread::new(..., Box::new(f))
} such that the Now, you have pub unsafe fn spawn_unchecked<F, T>(self, f: F) -> io::Result<JoinHandle<T>> where
F: FnOnce() -> T, F: Send, T: Send
{
imp::Thread::new(..., Box::new(f))
} which requires the Since If this is all too vague, happy to ping you about this again when my new target PR is ready. |
It seems to me that you have some misconceptions about lifetimes.
This seems to be at the core of it, as the I hope this explanations of the way I understand it makes sense to you. |
Most lenient from the perspective of the caller. I'm talking about the perspective of the callee. A function receiving |
Is there anything blocking this from stablization? |
Is there anything I can do to help this along to get stabilized? In my opinion the addition is straight forward, as it doesn't really alter the previous implementation and just exposes an intermediate step. If it helps, here is a link to a small demonstration how this function could be used to build a safe scoped thread abstraction that uses very little unsafe code (less than the current crossbeam implementation, which has to do lifetime transmutations). |
Bumping again, would like to see this stabilized. |
Would it make sense to also remove the |
I don't think so because it is never fine to send |
That is not correct, consider the following example: let rc = Rc::new(1);
let handle = unsafe { thread::spawn_unchecked(move || assert_eq!(*rc, 1)) };
handle.join().unwrap();
So I'd think it might be ok to remove the |
Ok, I'll adjust my statement. We already have an easy way to implement tldr: keeping the bound makes things more orthogonal, and easier to check |
Hello ! Any blockers for stabilisation ? Thanks a lot 🙂 |
Hello all, any updates on this? |
Per the stdlib developer's guide, here's a ping to @cuviper (found here) to see if this is ready for stabilization. I (on behalf of Adobe) need this stabilized for a concurrency library I'm developing. |
@rust-lang/libs-api: https://doc.rust-lang.org/nightly/std/thread/struct.Builder.html#method.spawn_unchecked Example of correct usage, adapted from https://github.com/asajeffrey/thread-init: use std::cell::RefCell;
use std::io;
use std::sync::mpsc::{self, Sender};
use std::thread::{self, JoinHandle};
// Allows initialization of a thread using borrowed data.
fn spawn_init<F, G, T>(f: F) -> io::Result<JoinHandle<T>>
where
F: FnOnce() -> G + Send,
G: FnOnce() -> T + 'static,
T: Send + 'static,
{
let (sender, receiver) = mpsc::channel();
let fg = move || {
struct Guard(Sender<()>);
impl Drop for Guard {
fn drop(&mut self) {
self.0.send(()).unwrap();
}
}
let guard = Guard(sender);
let g = f();
drop(guard);
g()
};
let thread_builder = thread::Builder::new();
let join_handle = unsafe { thread_builder.spawn_unchecked(fg) }?;
receiver.recv().unwrap();
Ok(join_handle)
}
fn main() {
thread_local! {
static LOCAL_STATE: RefCell<Vec<u8>> = panic!();
}
let mut shared_state = b"...".to_vec();
let thread = spawn_init(|| {
// access shared state
LOCAL_STATE.set(shared_state.clone());
|| {
// access only local state
LOCAL_STATE.with(|state| println!("{:?}", state));
}
})
.unwrap();
// shared state is no longer being accessed by the other thread
shared_state.clear();
thread.join().unwrap();
} Alternatives: let join_handle = thread_builder.spawn(unsafe {
mem::transmute::<
Box<dyn FnOnce() -> T + Send>,
Box<dyn FnOnce() -> T + Send + 'static>,
>(Box::new(fg))
})?; |
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The signature of |
Lifetime cleanup: #128745 |
…ubilee Remove unused lifetime parameter from spawn_unchecked Amanieu caught this when reviewing the stabilization proposal in rust-lang#55132. The `'a` lifetime here is useless. The signature is asking the caller of `spawn_unchecked` to "give me any lifetime that is shorter than your F's and T's lifetime", which they can always to with no effect, because arbitrarily short lifetimes exist.
Rollup merge of rust-lang#128745 - dtolnay:spawnunchecked, r=workingjubilee Remove unused lifetime parameter from spawn_unchecked Amanieu caught this when reviewing the stabilization proposal in rust-lang#55132. The `'a` lifetime here is useless. The signature is asking the caller of `spawn_unchecked` to "give me any lifetime that is shorter than your F's and T's lifetime", which they can always to with no effect, because arbitrarily short lifetimes exist.
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
Rollup merge of rust-lang#129161 - dtolnay:spawnunck, r=Noratrieb Stabilize std::thread::Builder::spawn_unchecked Closes rust-lang#55132.
This issue is for tracking the
thread_spawn_unchecked
feature (PR #55043).The feature adds an unsafe method to
thread::Builder
calledspawn_unchecked
, which allows spawning a thread without restrictions on the lifetimes of either the supplied closure or its return type. The method is unsafe because the caller has to guarantee that the spawned thread is explicitely joined before any referenced data is dropped.The main use for this feature is as building block for libraries that build safe abstractions around it, such as scoped threads.
Currently such libraries have to rely on subversions of the type system/borrow checker.
The text was updated successfully, but these errors were encountered: