4

I have a class defined like this: This is not all complete and probably won't compile.

class Server
{
public:
  Server();
  ~Server();

  class Worker
  {
  public: 
    Worker(Server& server) : _server(server) { }
    ~Worker() { }

    void Run() { }
    void Stop() { }

  private:  
    Server& _server;
  }


  void Run()
  {
    while(true) {
       // do work
    }
   }

  void Stop()
  {
     // How do I stop the thread?
  }

private:
  std::vector<Worker> _workers;
};

My question is, how do I initialize the workers array passing in the outer class named Server.

What I want is a vector of worker threads. Each worker thread has it's own state but can access some other shared data (not shown). Also, how do I create the threads. Should they be created when the class object is first created or externally from a thread_group.

Also, how do I go about shutting down the threads cleanly and safely?

EDIT:

It seems that I can initialize Worker like this:

Server::Server(int thread_count) 
  : _workers(thread_count), Worker(*this)), _thread_count(thread_count) { }

And I'm currently doing this in Server::Run to create the threads.

boost::thread_group _threads; // a Server member variable 

Server::Run(){
   for (int i = 0; i < _thread_count; i++)
     _threads.create_thread(boost::bind(&Server::Worker::Run, _workers(i)));

   // main thread.
   while(1) {  
       // Do stuff
   }

   _threads.join_all();
}

Does anyone see any problems with this? And how about safe shutdown?

EDIT: One problem I have found with it is that the Worker objects don't seem to get constructed! oops. Yes they do I need a copy constructor on the Worker class.

But oddly, creating the threads results in the copy constructor for Worker being called multiple times.

hookenz
  • 36,432
  • 45
  • 177
  • 286
  • Where does the input queue come from? How many workers do you want to create? – Björn Pollex Nov 09 '10 at 10:15
  • I've removed the input queue. I think it confuses things. The number if workers I want to create is variable and comes from a configuration file. – hookenz Nov 09 '10 at 10:20
  • Do you really need one class to be defined inside the other? It only makes it less readable to me. – Daniel Daranas Nov 09 '10 at 10:25
  • I'll leave your question to more boost-savvy people. Just thought I'd point out that the line 'Worker(Server& server) : _server(&server) { }' is wrong. Should be 'Worker(Server& server) : _server(server) { }' – manneorama Nov 09 '10 at 10:28
  • No, but the inner class needs to access the outer class members so if it's external then I need to make it a friend. The inner class will only ever be used but the outer class. – hookenz Nov 09 '10 at 10:28
  • Isn't server going to be a single intance with multiple workers ? Shouldn't you go the other way round, server inside worker ? – DumbCoder Nov 09 '10 at 10:31
  • You might think that, but this is a thread pool. The class I call server pulls UDP packets off the network and passes them into workers. – hookenz Nov 09 '10 at 10:36
  • I think the actual threads shouldn't be created by the Worker constructor but when when I call Server::Run. Otherwise, if during thread creation the constructor fails I might be bad. Don't want a thread running accessing Server which didn't get created. – hookenz Nov 09 '10 at 10:39
  • Thanks for all the feedback. I actually got it all working in the end. And believe me when I tell you the separate inner object is easier to understand and much cleaner than what I had before. – hookenz Nov 09 '10 at 12:23

2 Answers2

1

I have done it with pure WINAPI, look:

#include <stdio.h>
#include <conio.h>
#include <windows.h>
#include <vector>

using namespace std;

class Server
{

public:

    class Worker
    {
        int     m_id;
        DWORD   m_threadId;
        HANDLE  m_threadHandle;
        bool    m_active;

        friend Server;

    public:

        Worker (int id)
        {
            m_id = id;
            m_threadId = 0;
            m_threadHandle = 0;
            m_active = true;
        }

        static DWORD WINAPI Run (LPVOID lpParam)
        {
            Worker* p = (Worker*) lpParam;      // it's needed because of the static modifier

            while (p->m_active)
            {
                printf ("I'm a thread #%i\n",  p->m_id);
                Sleep (1000);
            }

            return 0;
        }

        void Stop ()
        {
            m_active = false;
        }
    };

    Server ()
    {
        m_workers = new vector <Worker*> ();
        m_count = 0;
    }

    ~Server ()
    {
        delete m_workers;
    }

    void Run ()
    {
        puts ("Server is run");
    }

    void Stop ()
    {
        while (m_count > 0)
            RemoveWorker ();

        puts ("Server has been stopped");
    }

    void AddWorker ()
    {
        HANDLE  h;
        DWORD   threadId;

        Worker* n = new Worker (m_count ++);
        m_workers->push_back (n);

        h = CreateThread (NULL, 0, Worker::Run, (VOID*) n, CREATE_SUSPENDED, &threadId);
        n->m_threadHandle = h;
        n->m_threadId = threadId;
        ResumeThread (h);
    }

    void RemoveWorker ()
    {
        HANDLE  h;
        DWORD   threadId;

        if (m_count <= 0)
            return;

        Worker* n = m_workers->at (m_count - 1);
        m_workers->pop_back ();

        n->Stop ();
        TerminateThread (n->m_threadHandle, 0);

        m_count --;

        delete n;
    }

private:

    int                 m_count;
    vector <Worker*>*   m_workers;
};

int main (void)
{
    Server  a;
    int     com = 1;

    a.Run ();

    while (com)
    {
        if (kbhit ())
        {
            switch (getch ())
            {
            case 27:        // escape key code

                com = 0;
                break;

            case 'a':       // add worker

                a.AddWorker ();
                break;

            case 'r':       // remove worker

                a.RemoveWorker ();
                break;

            }
        }
    }

    a.Stop ();

    return 0;
}

There are no synchronization code here, because I haven't enougth time to do it... But I wish it will help you =)

0

Have you looked at boost asio at all? It looks like it could be a good fit for what you are trying to do. Additionally you can call boost asio's io_service's run (similar to your Run method) from many threads i.e. you can process your IO in many threads. Also of interest could be http://think-async.com/Asio/Recipes for an asio based thread-pool.

Have a look at the asio examples. Perhaps they offer an alternative way of handling what you are trying to do. Esp. have a look at how a clean shutdown is accomplished.

Ralf
  • 9,405
  • 2
  • 28
  • 46