desync icon indicating copy to clipboard operation
desync copied to clipboard

Getting Rid of Send Requirement

Open anlumo opened this issue 4 years ago • 1 comments

Hello,

Correct me if I'm wrong, but it looks to me like the only reason why the type stored in the Desync has to be Send is that it's passed to its own desync thread in Desync::new. What if there would be an alternative function to construct a Desync using an 'static + Send + FnOnce() -> T that is called on that thread to create the wrapped object?

I currently have the problem that I need to use a type that's neither Send nor Sync from multiple threads. Since I can't use Desync, this is the quick replacement I could come up with:

use std::sync::mpsc;

struct NonSendDesync<T: 'static>(
    Option<(
        mpsc::Sender<Box<dyn 'static + Send + FnOnce(&mut T)>>,
        std::thread::JoinHandle<()>,
    )>,
);

impl<T: 'static> NonSendDesync<T> {
    pub fn new(f: impl 'static + Send + FnOnce() -> T) -> Self {
        let (sender, receiver): (mpsc::Sender<Box<dyn 'static + Send + FnOnce(&mut T)>>, _) = mpsc::channel();
        let join_handle = std::thread::spawn(move || {
            let mut instance = f();
            while let Ok(f) = receiver.recv() {
                f(&mut instance);
            }
        });

        Self(Some((sender, join_handle)))
    }

    pub fn enqueue(&self, f: impl 'static + Send + FnOnce(&mut T)) {
        self.0.as_ref().unwrap().0.send(Box::new(f)).unwrap();
    }
}

impl<T: 'static> Drop for NonSendDesync<T> {
    fn drop(&mut self) {
        if let Some((sender, join_handle)) = self.0.take() {
            drop(sender);
            join_handle.join().unwrap();
        }
    }
}

anlumo avatar Aug 02 '20 21:08 anlumo

This isn't possible with the current design: I think it would require two different types to implement this. It's quite in line with what desync sets out to achieve, though, so this is a worthy feature suggestion.

Right now, desync schedules tasks using a thread pool and by thread stealing, so the data inside a Desync is passed between threads a lot, and must be Send as a result.

It would be possible to schedule everything on a single thread always, which would eliminate the Send requirement, but I can't think of a way to combine that functionality into the exisitng type, so I think it would need to be a separate type. There might be some configuration to do, too: presumably it would be useful for the !Send types to share a thread most of the time to keep the thread count under control, but also to be able to be grouped into separate threads so they don't always block each other.

Finally, I think it might be necessary for sync to be able to schedule pending tasks for the same thread to prevent deadlocks when calling to Desyncs that use the thread pool or are single-threaded in other threads.

So it's doable but not 100% trivial. I think a single-threaded scheduler to go with the existing multi-threaded one is the first step - I think this is actually a lot simpler than the existing one in many respects.

Logicalshift avatar Aug 16 '20 20:08 Logicalshift