Reusing thread from vector

[hasrule]

Hi,

I was working on thread pool, which I used std::vector to implement the pool.

private:
    std::vector <handlerthread*> _pool;
    handlerthread* temp; 


void threadpool::addWorker() {
    handlerthread *temp;
    temp = new handlerthread(this);

    int x=_pool.size();
    temp->start(&x);
    _pool.push_back(temp);
    
}

handlerthread* threadpool::getIdleWorker(){
    int idx=-1;

    for (int i=0;i<_pool.size();i++){
        temp=(handlerthread*)_pool[i];   //SIGSEGV HERE
        if(temp->isIdle()){
           idx=i;
           break;
        }
    }
    
    if (idx < 0) {
        printf("need more worker!");
        addWorker();
        idx = _pool.size() - 1;
    }

    return _pool[idx];

}

Segmentation fault (SIGSEGV) is always received when I take back the thread from the vector.

what I want is to choose and use previously created thread, not creating a new one by copying the thread previously created. So I thought I should use

vector <handlerthread*> _pool

rather than

vector <handlerthread> _pool

.

Please give me some light.

Thanks a bunch!

[mike_2000_17]

Your problem is with handlerthread. Are you sure that once a thread is started that it remains valid when going back to idle or when it is initially at idle? This seems to mean that the handlerthread pointer is destroyed, or its internal handle to the actual OS thread (pthread or whatever) goes invalid (i.e. terminates). So, the code of your handlerthread would be more relevant to the problem you are getting, because there is (almost) nothing wrong with your vector implementation (and vector<handlerthread*> is correct, as long as you have the code which deletes all the threads before _pool gets deleted, that would be in the destructor of threadpool).

The one thing that could possibly also cause the trouble, depending on the code that uses threadpool (which I assume is a singleton), is that you are not protecting _pool with a mutex. Look up what a mutex is and why it is almost indispensable in multi-threaded code.

BTW, vector<handlerthread> is also possible to avoid dealing to much with pointers, but you need to write a proper copy-constructor (which needs to be a move-constructor) for your handlerthread class.

[hasrule]

o0ops, you re right nothing wrong with that code. I just forgot to assign the threadpool to the object that use the pool. So, it's SIGSEV :D

anyway, your reply would really help me in further development.

[hasrule]

anyway, is a move-constructor just copy-and-delete-constructor?

[mike_2000_17]

>>anyway, is a move-constructor just copy-and-delete-constructor?
when you have a class like handlerthread which, I assume, has an internal handle to the thread that it is running (windows HANDLE or posix pthread* or boost::thread), you don't want to copy it naively (default copy-constructor), because it makes no sense to have two handlerthread objects referencing the same thread of execution. So a move-constructor, is just a copy-constructor (with a non-const reference as parameter) that copies the content of its parameter and sets the parameter into a null state (like making the internal handle = NULL). In other words, the ownership of the resources is transfered (or moved) from one object to the other during the "copy" operation. Otherwise, if an object owns resources (like a thread handle) it should not be copyable if it's not movable instead. So a move-constructor is not a "copy-and-delete-constructor", but more a "ownership-transfer-constructor", such that ownership of a resource is always unique (the "owner" is, simply put, the object that is responsible for cleaning-up the resource, and you don't want to ever have more than one).

Anyways, glad to see you found the solution to your problem!