1
struct taskinfo
{
        long int id;
        bool cancel;
        std::function<void()> func;
        std::chrono::system_clock::time_point time;
        std::chrono::system_clock::duration interval;
        taskinfo(){ }

        bool operator<(const taskinfo& task) const {
            return time > task.time;
        }

        public:
        taskinfo(long int id, std::function<void()>&& f, const std::chrono::system_clock::time_point& t)
                : id(id), func(f),
                time(t)
        {
                cancel = false;
        }
}

....

std::priority_queue<taskinfo, std::vector<taskinfo>> tasks;
void at(taskinfo** task){
        std::function<void()> threadFunc = [task]() { std::thread((*task)->func).detach(); };
        (*task)->func = threadFunc;
        tasks.push(**task);
}

In main()..

std::vector<taskinfo*> requests;
for(int i=1; i <=5; i++ )
{
        taskinfo* t = new taskinfo(i, [i]{ timeoutFunc(i); }, std::chrono::system_clock::now() + std::chrono::milliseconds(timeout));
        tT.at(&t);
        requests.push_back(t);
        std::cout << "Request " << i << " Registered.... Time:" << std::chrono::system_clock::now().time_since_epoch().count() << std::endl;
}

I think I am missing something here when I pop the function out of the queue to execute, the function may be empty, Nothing is getting executed.

If i copy taskinfo to locally 

void at(taskinfo** task){
            taskinfo t = **task;
            //Replace everything else with t function works fine But 
            //I need to modify the same reference
}

How can I work with pointer reference herein lambda?

I have added the complete code of what i am trying to do here.

Complete Code: 

#include <functional>
#include <chrono>
#include <future>
#include <queue>
#include <thread>
#include <memory>
#include <sstream>
#include <assert.h>
#include <iostream>
#include <ctime>
#include <sys/time.h>
#include <unistd.h>
#include <limits.h>

#define TIMER_NO_TASK_SLEEP_TIME        100

struct taskinfo
{
        long int id;
        bool cancel;
        std::function<void()> func;
        std::chrono::system_clock::time_point time;
        std::chrono::system_clock::duration interval;

        taskinfo(){ }
        bool operator<(const taskinfo& task) const {
                return time > task.time;
        }

        public:
        taskinfo(long int id, std::function<void()>&& f, const std::chrono::system_clock::time_point& t)
                : id(id), func(f),
                time(t)
        {
                cancel = false;
        }
};

class TimerTask
{
        private:
                std::priority_queue<taskinfo, std::vector<taskinfo>> tasks;
                std::unique_ptr<std::thread> thread;
                bool keepRunning;

        public:
                TimerTask()
                        :keepRunning(true),
                        thread(new std::thread([this]() {
                                                while(keepRunning)
                                                {
                                                auto now = std::chrono::system_clock::now();
                                                while(!tasks.empty() && tasks.top().time <= now) {
                                                if(!tasks.top().cancel)
                                                {
                                                tasks.top().func();
                                                }
                                                tasks.pop();
                                                }

                                                if(tasks.empty()) {
                                                std::this_thread::sleep_for(std::chrono::milliseconds(TIMER_NO_TASK_SLEEP_TIME));
                                                } else {
                                                std::this_thread::sleep_for(tasks.top().time - std::chrono::system_clock::now());
                                                }
                                                }
                                                })){ }

                ~TimerTask()
                {
                        keepRunning = false;
                        thread->join();
                }

                //Execute a task when the timer times out
                void at(taskinfo** task){
                        std::function<void()> threadFunc = [task]() { std::thread((*task)->func).detach(); };
                        (*task)->func = threadFunc;
                        tasks.push(**task);
                }

                //Cancel the particular task with a flag
                void cancel(taskinfo** task){
                        (* task)->cancel = true;
                }
};

//The return type of the task must be void
void timeoutFunc(int id)
{
        std::cout << "Request " << id << " Timeout.... Executed Timeout Function Time:" << std::chrono::system_clock::now().time_since_epoch().count() << std::endl;
}

int main(int argc, char* argv[])
{
        if(argc != 2)
        {
                std::cout << "\n Usage <Process> <Timeout>" << std::endl;
                return 0;
        }

        int timeout = atoi(argv[1]);
        TimerTask tT;

        std::vector<taskinfo*> requests;
        requests.reserve(1000);
        for(int i=1; i <=5; i++ )
        {
                taskinfo* t = new taskinfo(i, [i]{ timeoutFunc(i); }, std::chrono::system_clock::now() + std::chrono::milliseconds(timeout));
                tT.at(&t);
                requests.push_back(t);
                std::cout << "Request " << i << " Registered.... Time:" << std::chrono::system_clock::now().time_since_epoch().count() << std::endl;
        }
        while(1) sleep(60);
        return 0;
}
raghu
  • 95
  • 1
  • 10
  • 2
    Looks like the problem is not with the lambda but with the storage duration of `**task`. – nwp Jan 30 '18 at 10:25
  • there is no pointer reference, or do I miss something? or am I just to picky about pointers strictly speaking not being references (in the C++ reference sense) ? – 463035818_is_not_an_ai Jan 30 '18 at 10:27
  • 1
    `taskinfo**` is a bad omen. I can't remember when I last saw a double pointer in proper C++ code. That said, there is no pointer reference in the lambda. `task` is captured _by value_`. – MSalters Jan 30 '18 at 10:28
  • I have updated function call and taskinfo initialization – raghu Jan 30 '18 at 10:33
  • @MSalters yes, i am not clear whether passing by value as `[task]` is right!? – raghu Jan 30 '18 at 10:42
  • The error probably comes from you storing your `taskinfo *`s in a `std::vector`. Every time you call `requests.push_back(t);` the `vector` might need to reallocate, meaning all the `taskinfo **` that you had stored in your lambda are now invalid. Quick way to check: Add `requests.reserve(1000);` before any `push_back`. – nwp Jan 30 '18 at 10:44
  • 1
    Storing a pointer to a local variable (`tT.at(&t);`) for later use is never a good idea. What do you want the extra indirection for? It seems to only be there in order to make the abundance of asterisks compile. – molbdnilo Jan 30 '18 at 10:46
  • I added but still `tasks.top().func();` doen't execute anything. – raghu Jan 30 '18 at 10:47
  • I'm not sure that a lambda with a capture can be copied to a `std::function`. I'm sure I've seen something about this on SO, but can't find it today. – quamrana Jan 30 '18 at 10:50
  • @molbdnilo Its a different class, I pass the pointer reference to the task using `at(...)` from any class. – raghu Jan 30 '18 at 10:52
  • @quamrana That works fine. What you can't do is convert a capturing lambda to a regular function pointer. – molbdnilo Jan 30 '18 at 10:52
  • @raghu That reason makes no sense. Just pass a pointer to the task. There's no point in passing a pointer to a pointer that will cease to exist almost immediately. (`&` in an expression does not mean "reference", it's the "address-of" operator.) – molbdnilo Jan 30 '18 at 10:54
  • I am using task info to execute a task at some point in time, if I need to cancel the task, I need the reference to the task, by copying I will get a new copy of the task, I can never cancel the task. – raghu Jan 30 '18 at 10:56
  • I took your code and ran it under [ASan](https://clang.llvm.org/docs/AddressSanitizer.html) which produced [the output at the bottom](http://coliru.stacked-crooked.com/a/1a9b664093983066). It shows a memory corruption. See if you can make sense of it and consider using ASan yourself. – nwp Jan 30 '18 at 11:13

1 Answers1

1

You are passing a pointer to a pointer which no longer exists:

taskinfo* t = new taskinfo(i, [i]{ timeoutFunc(i); }, std::chrono::system_clock::now() + std::chrono::milliseconds(timeout));
tT.at(&t);
requests.push_back(t);

In the above code t is a local variable instantiated for each iteration through the loop. You get a new t every time.

The code tT.at(&t); gets the address of this temporary.

The fix is at the calling site call: tT.at(t);. notice how this is just like requests.push_back(t);

Also:

//Execute a task when the timer times out
void TimerTask::at(taskinfo* task){
        std::function<void()> threadFunc = [task]() { std::thread(task->func).detach(); };
        task->func = threadFunc;
        ...
}
quamrana
  • 37,849
  • 12
  • 53
  • 71