1

so I am trying to create a queue that stores functions to be called later. In order to do this I must create a std::function object to place into the queue from any function passed. The issue is coming where I create this object and it can be called, but it seems the passed parameters arent being stored properly (how I want them to be)

The main issue is all inside the member template function push(Ret (&)(...), &&...) I tried inserting a call to the temp created function above the push(temp) and everything worked as expected. But when I try and access this function off of the queue it seems my parameters forwarded have been overriden.

class thread_queue {

  public:
    thread_queue() = default;
    void push(std::function<void()> func) { thread_q_.push(func); } 

    template <typename Ret, typename ... Params>
    void push(Ret (&func)(Params...), Params&&... params) {
      std::function<void()> temp = [&]() { // TO DO : FIX HERE
        func(std::forward<Params>(params)...);
      };
      push(temp);
    }

    void pop() { thread_q_.pop(); }

    std::function<void()> front() { return thread_q_.front(); }

  private:
    std::queue<std::function<void()>> thread_q_; 

};

void func1() {
  std::cout << "Inside of func1" << std::endl;
}

void func2(int a) {
  std::cout << "Inside of func2 " << a << std::endl;
}

int func3() {
  std::cout << "Inside of func3" << std::endl;
  return 5;
}

int func4(int a, int b) {
  std::cout << "Inside of func4 " << a + b << std::endl;
  return a + b;
}

int main() {

  thread_queue test;
  test.push(func1);
  test.push(func2, 10);
  test.push(func3);
  test.push(func4, 1, 8);

  test.front()();
  test.pop();
  test.front()();
  test.pop();
  test.front()();
  test.pop();
  test.front()();
  test.pop();


  return 0;
}

So with this i want to get

Inside of func1 Inside of func2 10 Inside of func3 Inside of func4 9 but instead I am getting Inside of func1 Inside of func2 8 Inside of func3 Inside of func4 9

Some further notes : I would like to try and forward the passed params so if i decide to pass some large object there will be less time wasted than copying it. I have also considered instead somehow using shared_ptr or unique_ptr on the parameters, but havent tested this as I would like to avoid this if possible. Thank you.

Edit: It seems my issue might have something to do with rvalue references being passed, because when I tried making 10, 1, and 8 into lvalues (by setting them as variables in main) it worked as expected. Looking more into this now

6
  • Not sure... but temp = [=, &params...]() ; or also temp = [&func, &params...]() ; unfortunately I cannot reproduce your problem. Commented Sep 1, 2019 at 20:22
  • Thank you for the input, when you say you cannot reproduce the problem, do you mean you are unable to access computer right now? or is it working on your computer? Commented Sep 1, 2019 at 20:31
  • No: I'm saying that I get "Inside of func2 10" and "Inside of func4 9" with your original code. Commented Sep 1, 2019 at 20:33
  • Don't we have race condition here? Although std::ofstream is threadsafe, multiple calls to the same object from different threads can get interleaved. Try locking a mutex while writing to std::cout. Commented Sep 1, 2019 at 22:48
  • There is no multithreading here, at least not that I know of. I am currently trying to make this queue for a later multithreading project . Commented Sep 2, 2019 at 3:31

1 Answer 1

1

Your queue is holding references to the arguments so the argument must still be in scope when the function is called. e.g.

{
   int value = 1;
   test.push(func2, value);
}
test.front()(); //Invalid, value is out of scope

int value2 = 2;
test.push(func2, value2);
test.front()(); //Ok, value2 is still in scope

test.push(func2, 3);
test.front()(); //Invalid, the temporary that was holding 3 is out of scope

If you want the function to always be valid you will need to store the arguments by value. Capturing parameter packs by value in a lambda isn't straight forward, however, we can use std::bind instead of a lambda.

#include <functional>
#include <queue>
#include <iostream>

class thread_queue {

  public:
    thread_queue() = default;
    void push(std::function<void()> func) { thread_q_.push(func); } 

    template <typename Ret, typename ... Params>
    void push(Ret (&func)(Params...), Params&&... params) {
      std::function<void()> temp = std::bind(func, std::forward<Params>(params)...);
      push(std::move(temp));
    }

    void pop() { thread_q_.pop(); }

    std::function<void()> front() { return thread_q_.front(); } //could avoid a copy 
      //by returning a reference. Would be more consistent with other containers.

  private:
    std::queue<std::function<void()>> thread_q_; 

};

void func1() {
  std::cout << "Inside of func1" << std::endl;
}

void func2(int a) {
  std::cout << "Inside of func2 " << a << std::endl;
}

int func3() {
  std::cout << "Inside of func3" << std::endl;
  return 5;
}

int func4(int a, int b) {
  std::cout << "Inside of func4 " << a + b << std::endl;
  return a + b;
}

int main() {

  thread_queue test;
  test.push(func1);
  test.push(func2, 10);
  test.push(func3);
  test.push(func4, 1, 8);

  test.front()();
  test.pop();
  test.front()();
  test.pop();
  test.front()();
  test.pop();
  test.front()();
  test.pop();


  return 0;
}

UPDATE: If you have move only parameters std::bind will not work as the object it returns can be called multiple times and thus can't move the stored parameters. Another problem with move only parameters is that std::function requires the function object passed to it to be copyable. One why of solving these problems is to store a std::shared_ptr in the std::function e.g.

#include <functional>
#include <queue>
#include <iostream>
#include <tuple>
#include <memory>

class thread_queue {
    template <typename Ret, typename... Params>
    struct callable {
        Ret (&func)(Params...);
        std::tuple<Params...> params;

        template<typename... Params2>
        callable(Ret (&func1)(Params...), Params2&&... params) :
            func(func1),
            params{std::forward<Params2>(params)...}
        {}

        void operator()() {
            std::apply(func, std::move(params));
        }
    };
  public:
    thread_queue() = default;
    void push(std::function<void()> func) { thread_q_.push(std::move(func)); } 

    template <typename Ret, typename... Params>
    void push(Ret (&func)(Params...), Params&&... params) {
         auto data = std::make_shared<callable<Ret, Params...>>(func, std::forward<Params>(params)...);
      thread_q_.push(std::function<void()>{
                [data = std::move(data)]() {
                    (*data)();
                }
            });
    }

    void pop() { thread_q_.pop(); }

    std::function<void()>& front() { return thread_q_.front(); }
  private:
    std::queue<std::function<void()>> thread_q_; 

};

struct MoveOnly {
    MoveOnly() {}
    MoveOnly(MoveOnly&&) {}
};

void func5(MoveOnly m) {
    std::cout << "func5\n";
}

int main() {
    thread_queue test;
    test.push(func5, MoveOnly{});
    test.front()();
    test.pop();

    return 0;
}

Another likely faster solution is to write your own version of std::function. The following is a minimal example of this and doesn't include small buffer optimization.

#include <functional>
#include <queue>
#include <iostream>
#include <tuple>
#include <memory>

template<class T>
class move_only_function;

template<class R, class... Args>
class move_only_function<R(Args...)>
{
    struct base_callable
    {
        virtual R operator()(Args... args) = 0;
        virtual ~base_callable() = default;
    };

    template<class F>
    struct callable : public base_callable
    {
        F func;
        callable(const F& f) : func(f) {}
        callable(F&& f) : func(std::move(f)) {}

        virtual R operator()(Args... args) override
        {
            return static_cast<R>(func(args...));
        }
    };

    std::unique_ptr<base_callable> func;
public:
    move_only_function(move_only_function&& other) : func(std::move(other.func)) {}

    template<class F>
    move_only_function(F&& f) : func(std::make_unique<callable<F>>(std::forward<F>(f))) {}

    template<class... Args2>
    R operator()(Args2&&... args)
    {
        return (*func)(std::forward<Args2>(args)...);
    }
};

class thread_queue {
  public:
    thread_queue() = default;
    void push(move_only_function<void()> func) { thread_q_.push(std::move(func)); } 

    template <typename Ret, typename ... Params>
    void push(Ret (&func)(Params...), Params&&... params) {
      thread_q_.push(move_only_function<void()>{
            [func, tup=std::make_tuple(std::forward<Params>(params)...)]() mutable {
                return std::apply(func, std::move(tup));
            }});
    }

    void pop() { thread_q_.pop(); }

    move_only_function<void()>& front() { return thread_q_.front(); }
  private:
    std::queue<move_only_function<void()>> thread_q_; 

};

struct MoveOnly {
    MoveOnly() {}
    MoveOnly(MoveOnly&&) {}
};

void func5(MoveOnly m) {
    std::cout << "func5\n";
}

int main() {
    thread_queue test;
    test.push(func5, MoveOnly{});
    test.front()();
    test.pop();

    return 0;
}
Sign up to request clarification or add additional context in comments.

1 Comment

I haven't implemented this yet to test it but will later tonight when I get the chance. I am kinda unfamiliar with bind and functionals and was kinda using this project im working on to get a hang of this stuff. So my question is, will this call the copy constructor for the params when we bind it to the function? Or will it perfect forward?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.