Skip to content
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

SYNC is not implemented for ThreadPool #96

Open
vnnv01 opened this issue Jul 23, 2018 · 5 comments
Open

SYNC is not implemented for ThreadPool #96

vnnv01 opened this issue Jul 23, 2018 · 5 comments

Comments

@vnnv01
Copy link

vnnv01 commented Jul 23, 2018

I am trying to create a threadpool as static. So that I dont have to pass this variable across multiple functions.

lazy_static! {
    pub static ref TABLES_LOAD_POOL: ThreadPool = setup_thread_pool();
}

fn setup_thread_pool () -> ThreadPool {
        
    let thread_pool :ThreadPool  = threadpool::Builder::new()
                      .num_threads(10)
                      .thread_name("Table_Load".into())
                      .build();
    thread_pool

}

I get the following error

| |_^std::sync::mpsc::Sender<std::boxed::Box<(dyn threadpool::FnBox + std::marker::Send + 'static)>> cannot be shared between threads safely
|

@dns2utf8
Copy link
Member

dns2utf8 commented Sep 6, 2018

Hi

Sorry this reply is so late. The reason we can not implement Sync is that the underlying channel does not implement it.
I solved the problem with a Mutex: https://play.rust-lang.org/?gist=cfd1222f7ba84f66d6338c7179a92656
If you use a local copy as in fn b then you don't loose too much speed from the mutex.
I will continue to think about that problem and hope to come up with a better solution.
If you already solved the problem, please let me know how you did it.

Cheers,
Stefan

@ghost
Copy link

ghost commented Sep 6, 2018

We should probably just put a Mutex around the Sender inside ThreadPool. Or use crossbeam-channel.

@dns2utf8
Copy link
Member

One requirement currently is that this crate compiles with rust 1.9.

@threema-danilo
Copy link

Just as a small bump, I'm also in a situation where a Sync threadpool would be useful. The execute method takes &self, so purely from an ownership point of view sharing it with an Arc (e.g. across multiple web request handlers) would be sufficient. However, this doesn't work because internally the ThreadPool isn't Sync.

@threema-danilo
Copy link

As a workaround, if it's fine to lock/unlock the mutex for every .execute() call, a wrapper like this can be used:

use std::sync::{Arc, Mutex};

use threadpool::ThreadPool;

/// A Send + Sync thread pool.
#[derive(Debug, Clone)]
pub struct SyncThreadPool {
    pool: Arc<Mutex<ThreadPool>>,
}

impl SyncThreadPool {
    /// Create a new thread pool with the specified size.
    pub fn new(num_threads: usize) -> Self {
        Self {
            pool: Arc::new(Mutex::new(ThreadPool::new(num_threads))),
        }
    }

    /// Execute a job on the thread pool.
    pub fn execute(&self, job: impl FnOnce() + Send + 'static) {
        self.pool
            .lock()
            .expect("could not lock thread pool mutex")
            .execute(job)
    }
}

This should have the same performance characteristics as if a mutex were used internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants