Priority Queue C++

Question

I created the following priority queue in C++

priority_queue < ThreadInfo*, vector<ThreadInfo*>, CompareThread > thread_queue;

where ThreadInfo class is

class ThreadInfo {
public:
    ThreadInfo();
    ThreadInfo(const ThreadInfo& orig);
    ThreadInfo(int thread_id,int init_time,int sleep_time,int run_time,int priority,int is_critical)
    {
        this->thread_id=thread_id;
        this->is_critical=is_critical;
        this->init_time=init_time;
        this->priority=priority;
        this->run_time=run_time;
        this->sleep_time=sleep_time;
    }

    void set_critical(bool value)
    {
        is_critical=value;
    }
    bool get_critical()
    {
        return is_critical;
    }
    void set_sleep_time(long value)
    {
        sleep_time=value;
    }

    long get_sleep_time(long value)
    {
        return sleep_time;
    }

    void set_run_time(long value)
    {
        sleep_time=value;
    }

    long get_run_time(long value)
    {
        return sleep_time;
    }
    int get_lock_type()
    {
        return lock_type;
    }
    void set_lock_type(int lock_type)
    {
        this->lock_type=lock_type;
    }

    int get_priority()
    {
        return priority;
    }
    void set_priority(int value)
    {
        this->priority=value;
    }

    unsigned long int get_thread_id()
    {
        return thread_id;
    }
    void set_thread_id(unsigned long int value)
    {
        this->thread_id=value;
    }
    virtual ~ThreadInfo();

private:
    unsigned long int thread_id;
    long init_time;
    long sleep_time;
    long run_time;
    int priority;
    bool is_critical;
    //1=spin,2=busy,3=semaphore
    int lock_type;



};

and the compare class is

class CompareThread {
public:
    bool operator()(ThreadInfo* th1, ThreadInfo* th2)
    {
       if (th1->get_priority()>th2->get_priority()) return true;

       return false;
    }
};

then I insert element in the following function,

void ThreadScheduler::register_thread(ThreadInfo &th)
{

        thread_queue.push(&th);


}

I call the thread register from the following function,

int ThreadController::thread_register(pthread_t &t, int priority, bool critical)
{
    ThreadInfo ti;
    cout<<"t reg:"<<t<<endl;
    ti.set_thread_id(t);
    ti.set_critical(critical);
    ti.set_priority(priority);
    ThreadScheduler::Instance()->register_thread(ti);
}

but each time when I push some threadinfo object to the queue I get the most recent object when I call the thread_queue.top(), whether it should return the thread object with lowest priority. Are there any problems here?

Answer 1

You are passing a pointer to the same memory chunk into the queue. You are calling the register_thread with a reference to a local object and queue its address. That is the reason they are all same. Another problem is, when you leave the thread_register function the local ti will be deleted (out of scope) and you will have no valid entries in the queue.

What you need to do is to allocate new memory for each info and copy the data into this memory. Thus, each element pointer you insert into the queue has to come from a different new, if you have a copy constructor this will do:

void ThreadScheduler::register_thread(ThreadInfo &th)
{
        thread_queue.push(new ThreadInfo(th));
        /* ... */
}

check this: https://stackoverflow.com/a/986093/390913

Answer 2

The problem is that the you are using the pointer to a variable that is declared locally in a function. As soon as the function (ThreadController::thread_register) is done the local variable does not exist anymore, and the pointer is now pointing at some unallocated memory.

There are two solutions to this:

1. Use smart pointers, like std::shared_ptr, and create a new pointer in ThreadController::thread_register:

   std::shared_ptr<ThreadInfo> ti(new ThreadInfo);
   

   Of course you have to remember to change to std::shared_ptr everywhere else too, and use the -> access operator instead of ..

2. Not use pointers at all, and let the class data (which is pretty minimal and simple) be copied.

I suggest going with alternative 2.