CODEWITHSUNDEEP
multithreadingstructrusttraitsunsafe
lncr
lncr

Reputation: 886

How to prevent autoimplementation of Sync

I have a struct containing unsafe code with the following method:

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

#[derive(Debug)]
struct Foo<T> {
    items: Vec<Box<(T, String)>>,
}

impl<T> Foo<T> {
    pub fn add_element(&self, element: T, key: String) {
        if !(self.items.iter().any( |i| i.1 == key)) {
            let mut items = unsafe {change_mut(&(self.items))};
            items.push(Box::new((element,key)));
        }
    }
}

unsafe fn change_mut<T>(x: &T) -> &mut T { // changes &self to &mut self
    &mut *(x as *const T as *mut T)
}

fn main() {
    let foo = Arc::new(Foo { items: vec!() });
    let clone = foo.clone();

    // This should not be possible, as it might lead to UB
    thread::spawn(move || clone.add_element(1, String::from("one")));

    println!("{:?}", *foo);

}

This struct is completely safe until someone starts using this method while multithreading. However, due to the fact the struct only contains a Vec<Box<T,String>>, Syncis implemented by default, which I would like to prevent.

I have found two ways to do this, both of which are not that great...

  1. Add a struct field which does not implement Sync for example *const u8, this is obviously rather bad, as it ends up resulting in unnecessary and unclear code and does not clearly show my intent.

  2. impl !Sync for Struct {} is not available on stable and will be removed according to this issue. The corresponding error is telling me to use marker types instead, but the documention does not present a way to solve my problem either.

error: negative trait bounds are not yet fully implemented; use marker types for
 now (see issue #13231)
  --> src\holder.rs:44:1
   |
44 | impl !Sync for Struct {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^

Upvotes: 4

Views: 1084

Answers (1)

Matthieu M.
Matthieu M.

Reputation: 299800

Interior mutability in Rust requires1 the use of UnsafeCell as a hint to the compiler that the normal rules do not apply.

Your structure should therefore look such:

#[derive(Debug)]
struct Foo<T> {
    items: UnsafeCell<Vec<Box<(T, String)>>>,
}

And then, the implementation of add_element is adjusted to:

impl<T> Foo<T> {
    pub fn add_element(&self, element: T, key: String) {
        if !(self.items.iter().any( |i| i.1 == key)) {
            let mut items = unsafe { &mut *self.items.get() };
            //                       ^~~~~~~~~~~~~~~~~~~~~~
            items.push(Box::new((element,key)));
        }
    }
}

The use of UnsafeCell makes change_mut completely unnecessary: it is the purpose of UnsafeCell, after all, to allow interior mutability. Note how its get method returns a raw pointer, which cannot be dereferenced without an unsafe block.

Since UnsafeCell does not implement Sync, Foo<T> will not implement Sync either, and therefore it becomes unnecessary to use negative implementations or any marker.

1 If you do not use it directly, chances are the abstractions you do use are built on it. It is as special as it could be: it is a lang item, as denoted by its attribute #[lang = "unsafe_cell"].

Upvotes: 5

Related Questions