CODEWITHSUNDEEP
c++winapithreadpool
unresolved_external
unresolved_external

Reputation: 2018

Thread pool is is taking all CPU resources

I've been working lately on my own thread pool implementation, and it works fine( pretty much). The problem is it takes almost all CPU resources. Please, take a look on my implementation:

ThreadPool.h

#include <vector>
#include <list>
#include <Windows.h>

const int DefaultThreadsNumber = 5;

class thread_pool
{
    typedef void ( *Task )( );

    void* m_hEvent;

    CRITICAL_SECTION m_hExecuteFunc;
    CRITICAL_SECTION m_hRunFunc;


    std::vector<void*> m_ThreadHandles;
    std::list<Task> m_TasksQueue;
    int m_nThreadInQueue;
    static unsigned __stdcall Run( void* thisParam );
public:
    thread_pool( int nMaxThreadNo );
    ~thread_pool();
    void Execute( Task task );
};

ThreadPool.cpp

#include "thread_pool.h"
#include <process.h>

void* g_hTaskFinishedEvent;

thread_pool::thread_pool( int nMaxThreadNo )
{
    if ( nMaxThreadNo <= 0 )
        nMaxThreadNo = DefaultThreadsNumber;

    m_nThreadInQueue = nMaxThreadNo;
    for( int i = 0; i < nMaxThreadNo; ++i )
    {
        m_ThreadHandles.push_back((void*)_beginthreadex(NULL, 0, &Run, (void*)this, 0, 0 ));
    }

    InitializeCriticalSection( &m_hExecuteFunc );
    InitializeCriticalSection( &m_hRunFunc );
}


thread_pool::~thread_pool()
{
    for ( std::vector<void*>::iterator it = m_ThreadHandles.begin(); 
            it != m_ThreadHandles.end(); 
            ++it )
    {
        CloseHandle( *it );
    }

    DeleteCriticalSection( &m_hExecuteFunc );
    DeleteCriticalSection( &m_hRunFunc );

}

void thread_pool::Execute( Task task )
{
    EnterCriticalSection( &m_hExecuteFunc );

    m_TasksQueue.push_back( task );

    LeaveCriticalSection( &m_hExecuteFunc );

    m_hEvent = CreateEvent(NULL, true, false, NULL);
    SetEvent( m_hEvent ); // TODO: what if setEvent will fail???
}

unsigned __stdcall thread_pool::Run(void* thisParam )
{
    thread_pool *This = (thread_pool*)thisParam;
    while(true)
    {
        WaitForSingleObject( This->m_hEvent, INFINITE );
        while(!This->m_TasksQueue.empty())
        {


            EnterCriticalSection( &This->m_hExecuteFunc );

            if ( !This->m_TasksQueue.empty() ) 
            {
                This->m_TasksQueue.front()();
                This->m_TasksQueue.pop_front();

                g_hTaskFinishedEvent = CreateEvent(NULL, true, false, NULL);
                SetEvent( g_hTaskFinishedEvent );
            }

            LeaveCriticalSection( &This->m_hExecuteFunc );
        }
    }
    return 0;
}

How can it be improved?

Thanks on advance.

Upvotes: 0

Views: 203

Answers (1)

Agent_L
Agent_L

Reputation: 5420

In CreateEvent, bManualReset is set to true, but you're not calling ResetEvent anywhere. Thus, once signalled, the event is remains set forever and WaitForSingleObject returns immediatelly. Make it an auto-reset event, or call ResetEvent after all workers are done.

Upvotes: 3

Related Questions