1

I am attempting to put together some example code from a book.. but the book and the github copy are different.. I think I am close to having a working example of a threadpool which accepts functions and wraps them so you wait for their value to return as a future... but getting compilation errors around templating

I've tried to instantiate the exact class I need at the end of the helloworld.cpp like told in

Why can templates only be implemented in the header file?

but then proceed to get a bunch of warnings about trying to imply a function which I had set as =delete already...

see what I mean under Why do C++11-deleted functions participate in overload resolution?

I'm a bit new to C++ still so unsure how to best proceed , compilation error is at the end.. I have already tested thread_safe_queue.cpp and it worked in other simpler usages already so I don't believe it's at fault here... more so the templating is needing help

helloworld.cpp

#include <thread>
#include <vector>
#include <atomic>
#include <iostream>
#include <future>
#include <functional>

#include <unistd.h>

 // https://github.com/anthonywilliams/ccia_code_samples/blob/6e7ae1d66dbd2e8f1ad18a5cf5c6d25a37b92388/listings/listing_9.1.cpp
 // https://github.com/anthonywilliams/ccia_code_samples/blob/6e7ae1d66dbd2e8f1ad18a5cf5c6d25a37b92388/listings/listing_9.2.cpp

#include "thread_safe_queue.cpp"

class function_wrapper
{
    struct impl_base {
        virtual void call()=0;
        virtual ~impl_base() {}
    };
    std::unique_ptr<impl_base> impl;
    template<typename F>
    struct impl_type: impl_base
    {
        F f;
        impl_type(F&& f_): f(std::move(f_)) {}
        void call() { f(); }
    };
public:
    template<typename F>
    function_wrapper(F&& f):
        impl(new impl_type<F>(std::move(f)))
    {}

    void call() { impl->call(); }

    function_wrapper(function_wrapper&& other):
        impl(std::move(other.impl))
    {}

    function_wrapper& operator=(function_wrapper&& other)
    {
        impl=std::move(other.impl);
        return *this;
    }

    function_wrapper(const function_wrapper&)=delete;
    function_wrapper(function_wrapper&)=delete;
    function_wrapper& operator=(const function_wrapper&)=delete;
};

struct join_threads
{
    join_threads(std::vector<std::thread>&)
    {}
};

class thread_pool
{
    std::atomic_bool done;
    thread_safe_queue<function_wrapper> work_queue;
    std::vector<std::thread> threads;
    join_threads joiner;

    void worker_thread()
    {
        while(!done)
        {
            std::function<void()> task;
            if(work_queue.try_pop(task))
            {
                task();
            }
            else
            {
                std::this_thread::yield();
            }
        }
    }
public:
    thread_pool():
        done(false),joiner(threads)
    {
        unsigned const thread_count=std::thread::hardware_concurrency();
        try
        {
            for(unsigned i=0;i<thread_count;++i)
            {
                threads.push_back(
                    std::thread(&thread_pool::worker_thread,this));
            }
        }
        catch(...)
        {
            done=true;
            throw;
        }
    }

    ~thread_pool()
    {
        done=true;
    }

    template<typename FunctionType>
    std::future<typename std::result_of<FunctionType()>::type>
    submit(FunctionType f)
    {
        typedef typename std::result_of<FunctionType()>::type result_type;
        
        std::packaged_task<result_type()> task(std::move(f));
        std::future<result_type> res(task.get_future());
        work_queue.push_back(std::move(task));
        return res;
    }
};

void find_the_answer_to_ltuae(){
    std::cout << "About to sleep 1 second...";
    sleep(1);
    std::cout<<"The answer is " << 42 << std::endl;
}

int main()
{
    std::cout << "About to create my_threadpool...." << std::endl;
    thread_pool my_threadpool;
    std::cout << "Done creating my_threadpool...." << std::endl;
    std::cout << "Submitting first job now" << std::endl;
    my_threadpool.submit(find_the_answer_to_ltuae);
    sleep(10);
    std::cout <<"Finished" << std::endl;
}

template class thread_safe_queue<function_wrapper>; // <-------- this was added by me, in an attempt to get the templating happy.. didn't work

thread_safe_queue.cpp

#include <mutex>
#include <memory>
#include <condition_variable>
#include <queue>

template <typename T>
class thread_safe_queue {
    public:
        // constructor
        thread_safe_queue() {}
        thread_safe_queue(const thread_safe_queue& other)
        {
            std::lock_guard<std::mutex> lock{other.mutex};
            queue = other.queue;
        }

        void push(T new_value)
        {
            std::lock_guard<std::mutex> lock{mutex};
            queue.push(new_value);
            cond.notify_one();
        }

        void wait_and_pop(T& value)
        {
            std::unique_lock<std::mutex> lock{mutex};
            cond.wait(lock, [this]{ return !queue.empty(); });
            value = queue.front();
            queue.pop();
        }

        std::shared_ptr<T> wait_and_pop()
        {
            std::unique_lock<std::mutex> lock{mutex};
            cond.wait(lock, [this]{ return !queue.empty(); });
            std::shared_ptr<T> res{std::make_shared<T>(queue.front())};
            queue.pop();
            return res;
        }

        bool try_pop(T& value)
        {
            std::lock_guard<std::mutex> lock{mutex};
            if (queue.empty())
                return false;
            value = queue.front();
            queue.pop();
            return true;
        }

        std::shared_ptr<T> try_pop()
        {
            std::lock_guard<std::mutex> lock{mutex};
            if (queue.empty())
                return std::shared_ptr<T>{};
            std::shared_ptr<std::mutex> res{std::make_shared<T>(queue.front())};
            queue.pop();
            return res;
        }

        bool empty() const
        {
            std::lock_guard<std::mutex> lock{mutex};
            return queue.empty();
        }

    private:
        mutable std::mutex mutex;
        std::condition_variable cond;
        std::queue<T> queue;
};


//https://stackoverflow.com/questions/495021/why-can-templates-only-be-implemented-in-the-header-file

Without the template class function_wrapper .. my compiler gives

sh compile.sh                                       
-- Conan: Adjusting output directories
-- Conan: Using cmake targets configuration
-- Conan: Adjusting default RPATHs Conan policies
-- Conan: Adjusting language standard
-- Current conanbuildinfo.cmake directory: /build
-- Configuring done
-- Generating done
-- Build files have been written to: /build
Scanning dependencies of target test_cpp_multi
[ 33%] Building CXX object CMakeFiles/test_cpp_multi.dir/src/helloworld.cpp.o
/src/helloworld.cpp:70:27: error: no matching member function for call to 'try_pop'
            if(work_queue.try_pop(task))
               ~~~~~~~~~~~^~~~~~~
/src/thread_safe_queue.cpp:41:14: note: candidate function not viable: no known conversion from 'std::function<void ()>' to 'function_wrapper &' for 1st argument
        bool try_pop(T& value)
             ^
/src/thread_safe_queue.cpp:51:28: note: candidate function not viable: requires 0 arguments, but 1 was provided
        std::shared_ptr<T> try_pop()
                           ^
/src/helloworld.cpp:113:20: error: no member named 'push_back' in 'thread_safe_queue<function_wrapper>'
        work_queue.push_back(std::move(task));
        ~~~~~~~~~~ ^
2 errors generated.
make[2]: *** [CMakeFiles/test_cpp_multi.dir/src/helloworld.cpp.o] Error 1
make[1]: *** [CMakeFiles/test_cpp_multi.dir/all] Error 2
make: *** [all] Error 2

1 Answer 1

1

For the fist error of

/src/helloworld.cpp:70:27: error: no matching member function for call to 'try_pop'
        if(work_queue.try_pop(task))
    

Your issue is that task is a std::function<void ()>, but try_pop expects a function_wrapper&. std::function<void ()> doesn't have an operator function_wrapper&, so you can't pass it to try_pop. What you would need to do is either change try_pop to take a const T&, so a temporary function_wrapper could be created, or create you own function_wrapper that wraps task and pass that to try_pop.

Your second error of

/src/helloworld.cpp:113:20: error: no member named 'push_back' in 'thread_safe_queue<function_wrapper>'
    work_queue.push_back(std::move(task));
    

is a typo. You don't have a push_back function in thread_safe_queue, but you do have a push function. You jus need to change

work_queue.push_back(std::move(task));

to

work_queue.push(std::move(task));
1
  • thanks @nathanoliver , I ended up changing the type of task to function_wrapper.. I think the book author messed up the code in git on purpose so you have to buy the book.. getting other errors but will play around. THANKS!
    – Erik
    Jan 12, 2021 at 19:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.