Somehow able to get rustc to accept mutiple mutable references

Hey,

I was trying to improve my project if have managed to write the following piece of code that will happily compile, even though it shouldn't:

        {
            let job_sys = JobSystem::new(2, 128).unwrap();
            let js = JobScope::new_from_system(&job_sys);
            let mut v: u32 = 0;
            for _ in 0..100 {
                let x = &mut v;
                let handle = js.create(move || {*x += 10}).unwrap(); // <-- problem here
                js.run(&handle).unwrap();
            }
        }

In the above snippet it is possible to create multiple closures with a mutable reference to v that will be run on worker threads once we reach js.run(..). At run time this would be a race condition. I've also tried replacing v with a Vec and I get the same result.

The handle is defined as:

pub struct ScopedJobHandle<'scope, T> {
    h: usize,
    p: PhantomData<T>
    s: PhantomData<&'scope mut ()>,
}

I tried adding a PhantomData<T> in an attempt to circumvent the above problem, but it has no effect on the above code.

and the function signature for create is

 pub fn create<T>(&self, job: T) -> Result<ScopedJobHandle<T>, Error>
    where
        T: Sized + FnOnce() + Send

Another thing I noticed is, if i try to store the handles in a Vec, it does indeed complain about this:

        let mut jobs = vec![];
        {
            let job_sys = JobSystem::new(2, 128).unwrap();
            let js = JobScope::new_from_system(&job_sys);
            let mut v: u32 = 0;
            for _ in 0..100 {
                let x = &mut v;
                let handle = js.create(move || {*x += 10}).unwrap(); // <-- problem here
                js.run(&handle).unwrap();
                jobs.push(handle); // <-- get error here about multiple mutable borrows of v 
            }
        }

Error:
    |
611 |                 let x = &mut v;
    |                         ^^^^^^ `v` was mutably borrowed here in the previous iteration of the loop

This in turn leads me to believe that PhantomMarker<T> on ScopedJobHandle will work if keep the handles alive during the execution of the job. That being said, I would also like to use this in a fire and forget sort of approach.

Am I missing something here? Shouldn't the trait requirement of Send prevent this in the first place?

You need to somehow tie the lifetime of T to the JobScope instead of relying on the return value being annotated. I think you want something like this:

pub fn create<T>(&self, job: T) -> Result<ScopedJobHandle<T>, Error>
    where
        T: Sized + FnOnce() + Send + 'jobsys

This requires that the JobScope can safely keep the T instance for as long as the JobSystem is borrowed.


Send only checks for synchronization; you need to also bound lifetimes to ensure that types aren’t dropped prematurely. In your present code, the type system believes that no references to v escape the body of the loop.

If I expected a problem, I wouldn't expect it here. Your handle doesn't have to last beyond the end of the loop, so the borrow can be made short enough to meet constraints. If there's a problem, it would be somewhere down the call graph where you attempted to store (in some sense) the &mut capturing closure (for a lifetime not known to meet the constraints).

I looked at your code a bit but gave up following it around before I found anywhere you attempted to send it across threads or indirectly store it with unsafe or anything, so I can't help with that, sorry.

&mut T is Send when T is Send.

1 Like

It looks like the type erasure occurs here and the unsafe write occurs here.


I think that the root problem is that Job::store assumes that the T it’s storing will remain valid for as long as the Job instance exists. The type system has no way to enforce this here unless you add a PhantomData<T> field to Job. Otherwise, it should be an unsafe fn. It’s then the callers’ responsibility to ensure that T can safely be kept for the entire life of the Job.

1 Like

That is a good point. Do you know whether it is possible to create a PhantomData<T> for a erasured type or is this simply beyond the scope of the language?

At a minimum, you'll need to have a lifetime parameter on Job if you want the compiler to help enforce this. Something like a PhantomData<dyn Send + 'a> field might be all you need. (This is probably equivalent to something simpler-- I'm no expert on lifetime and variance issues).

If that's not enough erasure for you, tag Job's methods as unsafe and enforce things manually at a higher level, where you can reason about the whole system's behavior.

I'll give that a go and hope for the best. Thanks :slight_smile:

In Rust &mut is primarily about being exclusive. If you don't want to enforce exlusivity, you don't want &mut. There are many other ways to mutate things.

In this case you need AtomicU32 which allows mutation from behind a shared reference.

You'll also need to make it Arc<AtomicU32> to give it shared ownership. & is bound to a single scope and temporary, and that usually makes it fundamentally incompatible with jobs/threads that are not bound to a scope.

2 Likes