Re: static and globals in a thread environment

Not at all. In fact, there is no reason whatsoever that, under the
scenario he proposed,
that any of the data needs to be global or static.

You sent me some email, but rather than respond privately, I'd rather post
the solution so
others can study it as well.

When you say "real time" you need to define what you mean by "real time".
What
granularity? Is it hard real time? What is your drop-dead failure
window?

I wouldn't generally waste time building circular buffers for feeding
multiple threads;
I'd put an I/O Completion Port in as a queue, shove packets into it using
PostQueuedCompletionStatus, and have a pool of threads doing
GetQueuedCompletionStatus.
All synchronization is handled by magic in the IOCP itself, and you don't
need any
explicit synchronization mechanism; furthermore, you don't worry about
running out of
elements. You can either preallocate a cache of elements and return them
to the cache
when you're done with them, or just use new/delete or malloc/free. Key
here is
performance issues.

Each time you collect a packet, you use PostQueuedCompletionStatus to put
it in the queue.
You can have as many threads as you want removing elements from the queue.
I've gotten
incredibly good performance out of this mechanism for a really
hard-real-time device, but
since you have already interposed complete network delays totaling perhaps
seconds, the
cost of the IOCP is going to be largely unnoticed.

However, no matter how you arrange it, a static or global variable would
not be required
in any implementation I can think of. You have a class which is your
queue manager, which
has AddTail and RemoveHead methods (one thing that always worries me is
that as soon as
someone says "circular queue" their head is usually in the wrong place
with respect to the
problem, since that is rarely an important consideration; it is common in
embedded
realtime systems because of very hard realtime constraints, but with your
machines
gathering data and sending it over TCP/IP, I seriously question that any
delays added by
simpler solutions could have any impact on the relatively massive delays
caused in a
single router, so just throw out the idea of circular buffer completely
and start at the
abstraction and work down).

class Queue {
    public:
         Queue();
         virtual ~Queue();
         void AddTail(Thing * p);
     Thing * RemoveHead();
         void Shutdown();
    protected:
         CList<Thing*> Queue;
         HANDLE sem;
         HANDLE shutdown;
         CRITICAL_SECTION lock;
         HANDLE handles[2];
class CQueueShutdownException: public CException {
       public:
          CQueueShutdownException: CException() { }
       }
class CQueueErrorException : public CException {
       public:
           DWORD GetError() { return error; }
           CQueueErrorException(DWORD err) : CException() { error = err; }
       protected:
           DWORD error;
      }
};

void Queue::AddTail(Thing * p)
   {
    EnterCriticalSection(&lock);
    Queue.AddTail(p);
    LeaveCriticalSection(&lock);
    ReleaseSemaphore(sem, 1, NULL);
   }

Thing * Queue::RemoveHead()
   {
    HANDLE handles[2];
    handles[0] = shutdown;
    handles[1] = sem;
    switch(WaitForMultipleObjects(handles, 2, FALSE, INFINITE))
       { /* wfmo */
        case WAIT_OBJECT_0:
             throw new CQueueShutdownException;
        case WAIT_OBJECT_0+1:
             break;
        default:
             throw new CQueueErrorException(::GetLastError());
       } /* wfmo */
     EnterCriticalSection(&lock);
     Thing * result = Queue.RemoveHead();
     LeaveCriticalSection(&lock);
     return result;
    } // Queue::RemoveHead

Queue::Queue()
  {
   InitializeCriticalSection(&lock);
   sem = ::CreateSemaphore(NULL, 0, MAX_ELEMENTS, NULL);
   shutdown = ::CreateEvent(NULL, FALSE, FALSE, NULL);
  }

Queue::~Queue()
  {
   DeleteCriticalSection(&lock);
   ::CloseHandle(sem);
  }

void Queue::Shutdown()
  {
   ::SetEvent(shutdown);
  }

...to invoke this

Queue * q = new Queue;

AfxBeginThread(handler, q);
AfxBeginThread(handler, q);
AfxBeginThread(handler, q);

/* static */ UINT CMyClass::handler(LPVOID p)
   {
    Queue * q = (Queue *)p;
    while(TRUE)
       { /* processing loop */
        try {
          Thing * t = q->RemoveHead();
          ...process element
          }
        catch(CQueueShutdownException * e)
          { /* shutdown */
           e->Delete();
           return 0;
          } /* shutdown */
        catch(CQueueErrorException * e)
          { /* error */
           DWORD err = e->GetError();
           ...deal with it
           e->Delete();
           return 0; // decision here is to terminate thread
          } /* error */
       } /* processing loop */
   } // CMyClass::handler

Note that there is not a single instance of a global or static variable in
the above. If
I wanted to use an IOCP, I would do it as

class Queue {
    public:
        void AddTail(Thing * p);
        Thing * RemoveHead();
        Queue();
        virtual ~Queue();
   protected:
        HANDLE iocp;
        class Reason {
            public:
                 typedef enum {AddElement, ShutDown} reason;
           };
        ... CQueueShutdownException, CQueueErrorException as in previous
class
};

Queue::Queue()
  {
   iocp = ::CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0);
  }

Queue::~Queue()
  {
   ::CloseHandle(iocp);
  }

void Queue::AddTail(Thing * p)
   {
    ::PostQueuedCompletionStatus(iocp, 0, Reason::AddElement,
(LPOVERLAPPED)p);
   }

Thing * Queue::RemoveHead()
  {
   DWORD bytesTransferred;
   UINT_PTR key;
   Thing * p;
   BOOL b = ::GetQueuedCompletionStatus(iocp, &bytesTransferred, &key,
(LPOVERLAPPED*)&p,
INFINITE);
   if(!b)
      throw new CQueueErrorException(::GetLastError());
   switch(key)
        {
         case Reason::AddElement:
              return p;
         case Reason::Shutdown:
              throw new CQueueShutdownException;
         default:
              throw new CQueueErrorException(ERROR_INVALID_PARAMETER);
        }
   } // Queue::RemoveHead

void Queue::Shutdown()
  {
   ::PostQueuedCompletionStatus(iocp, 0, Reason::Shutdown, NULL);
  }

Note that in this case there is not a single global or static variable
involved either,
and in fact the processing loop in the 'handler' thread does not change in
the slightest
with the change in implementation!

The more obvious error tests, minor syntax error cleanup, casts and/or
proper spelling of
various names, are left as an Exercise For The Reader.

I would choose the second implementation. If it had performance problems,
I could replug
it with the first implementation and measure the performance difference to
determine which
one mattered. But as long as the Internet is in between you and your
events, you are
potentially seconds behind realtime, and performance of either of these
would not be an
issue.

Last year I delivered 150KSLOC of C++ application and there was not a
single global or
static variable to be found in it. It is very clear that you can write
good code without
these. Also, there was not a single variable added to my CWinApp class
for the
application. Putting variables in the CWinApp-derived class is just
creating global
variables with syntactic saccharine (not even as good as syntactic sugar).
Not one module
I wrote contained a reference to the header file for the CWinApp class
(only the
CWinApp-derived class had this header file, the only instance in the
entire 400+ source
file set).
joe

On Wed, 21 Nov 2007 18:50:50 +0200, "Gilai" <gilai@intern.net> wrote:


Joseph M. Newcomer [MVP]
email: newcomer@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm