[Last Call] Learn about multicloud storage options and how to improve your company's cloud strategy. Register Now

x
?
Solved

Multithread Synchronization

Posted on 2006-11-21
8
Medium Priority
?
673 Views
Last Modified: 2008-03-10
Hi Experts,

Been a while since I've built a multithreaded application - put together an app that spawns off 300 threads and ofcourse I'm now runing into tons of fun synch issues. I've condensed my code as much possible. WaitForMultipleObjects also does not appear to work - the app just ends without waiting for all threads to finish.  Any help is greatly appreciated.

#include "fifo.h"

#define NUM_THREADS 300

typedef struct{

      int enabled;
      int data;
      int rate;
      FIFO         fifo;      
}Thread_Info_Type;


Thread_Info_Type thread_info[NUM_THREADS] = { 0 };
HANDLE                   thread_event_hdls[NUM_THREADS] = { 0 };


int terminate_flag      = 0;


CRITICAL_SECTION GlobalCriticalSection;


void Worker_Thread(int *i)
{
      int j = *i;
      int data = 0;

      Fifo_Init(&thread_info[j].fifo);
            
      while((!terminate_flag) || (!Is_Fifo_Empty(&thread_info[j].fifo)))
      {
            
            data = Fifo_Pop(&thread_info[j].fifo);      
            
            if(thread_info[j].enabled)
            {
                  printf("Thread %d\n",j);
            }

            Sleep(thread_info[j].rate);
      }

      Fifo_Terminate(&thread_info[j].fifo);
            
      SetEvent(thread_event_hdls[j]);
}


void Initialize_Threads()
{
      HANDLE ThreadHandle;      
      int      i = 0;

      InitializeCriticalSection(&GlobalCriticalSection);
            
      terminate_flag   = 0;

      for( i = 0; i < NUM_THREADS; i++)
      {            
            thread_event_hdls[i] = CreateEvent ( NULL, FALSE, FALSE, NULL);
            thread_info[i].rate  = 20;
                               thread_info[j].enabled = 1;

            ThreadHandle = CreateThread(NULL,
                                          16000,
                                          (LPTHREAD_START_ROUTINE)Worker_Thread,
                                          &i,
                                          0,
                                          0);  
            
            if (ThreadHandle == NULL){      
                  MessageBox(0,"Error Creating Thread!", "Initialize Threads Error!", MB_OK);       
            }            
      
            SetThreadPriority(ThreadHandle,THREAD_PRIORITY_NORMAL);
      }
}

void Terminate_Threads()
{
      int i = 0;

      EnterCriticalSection(&GlobalCriticalSection);
      terminate_flag = 1;
      LeaveCriticalSection(&GlobalCriticalSection);

      WaitForMultipleObjects(NUM_THREADS,thread_event_hdls,TRUE, INFINITE);

      MessageBox(0,"DONE With ALL Threads!", "Terminate Done!", MB_OK);

      DeleteCriticalSection(&GlobalCriticalSection);
}


void main( )
{
      int i = 0;

      Initialize_Threads();
      
      

      for(i = 0; i < NUM_THREADS*5; i++){
      Fifo_Push(&thread_info[i%NUM_THREADS].fifo,i);
      }

      Sleep(1000);
      
      Terminate_Threads();
      
            
}


///////////////////////////////////
fifo
//////////////////////////////////
int Fifo_Init(FIFO *fifo)
{
      ELEMENT *e;
      
      InitializeCriticalSection(&fifo->critical_section);

      EnterCriticalSection(&fifo->critical_section);

      e = malloc(sizeof(ELEMENT));
      
      if( e == NULL ){
            DeleteCriticalSection(&fifo->critical_section);
            MessageBox(NULL,"Malloc Failed!","Fifo_Init Error!",MB_OK);
            return -1;
      }
      e->next = NULL;
      e->data = 0;
      fifo->first = e;
      fifo->last = e;

      LeaveCriticalSection(&fifo->critical_section);

      return 1;
}

void Fifo_Terminate(FIFO *fifo) {
      
      ELEMENT *e, *e_temp;

      EnterCriticalSection(&fifo->critical_section);
      
      e = fifo->first;

      while ( e != NULL ) {
      
            e_temp = e->next;
            free(e);
            e = e_temp;            
      }

      fifo->first = NULL;
      fifo->last  = NULL;

      LeaveCriticalSection(&fifo->critical_section);

      DeleteCriticalSection(&fifo->critical_section);

}
int Fifo_Push(FIFO *fifo,CEI_UINT32 data) {
      
      ELEMENT *e;

      EnterCriticalSection(&fifo->critical_section);

      if( fifo->last == NULL ) {            
            MessageBox(NULL,"Fifo Not Initialized!","Fifo_Push Error!",MB_OK);
            return -1;
      }

      e = malloc(sizeof(ELEMENT));
      
      if( e == NULL ){
            MessageBox(NULL,"Malloc Failed!","Fifo_Push Error!",MB_OK);
            return -1;
      }

      e->data = 0;
      e->next = NULL;
      fifo->last->data = data;
      fifo->last->next = e;
      fifo->last = e;

      LeaveCriticalSection(&fifo->critical_section);

      return 1;
}
CEI_UINT32 Fifo_Pop(FIFO *fifo) {
      
      ELEMENT    *e;
      CEI_UINT32 data;

      EnterCriticalSection(&fifo->critical_section);

      if( fifo->last == fifo->first )
            return fifo->first->data;
      
      e = fifo->first;
      fifo->first = fifo->first->next;
      data = e->data;
      free(e);

      LeaveCriticalSection(&fifo->critical_section);

      return data;
}
int Is_Fifo_Empty(FIFO *fifo) {

      int ret_val;

      EnterCriticalSection(&fifo->critical_section);

      ret_val = fifo->last == fifo->first ? 1 : 0;

      LeaveCriticalSection(&fifo->critical_section);

      return ret_val;

}
0
Comment
Question by:DizZzM
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 3
  • 3
  • 2
8 Comments
 
LVL 48

Accepted Solution

by:
AlexFM earned 2000 total points
ID: 17993711
Your way to pass &i to thread is not safe. Pass i by value:

ThreadHandle = CreateThread(NULL,
                                   16000,
                                   (LPTHREAD_START_ROUTINE)Worker_Thread,
                                   (void*)i,   // <<
                                   0,
                                   0);  

void Worker_Thread(void* p)
{
     int j = (int) p;
     ...
}

To wait for threads, you can use thread handles and not thread_event_hdls array. Thread handle is signaled when thread exits, so you can wait directly for array of thread handles returned by CreateThread.
0
 

Author Comment

by:DizZzM
ID: 17993865
Thanks for the prompt response Alex -  your suggestion actually fixed a few things. Stopped getting assertion errors and the thread signalling now appears to be working. Appears the indexing was inconsistent the way I had it coded - good catch. I thought *i would copy the value to j and that value would stick until the thread exited but I guess this isn't the case? Mind explaining?

Still having synch issues though - the issue appears to be with my fifo. If I just let the worker thread run and pop items off the fifo all threads exit nicely. If I  put in the following code in main though:

     for(i = 0; i < NUM_THREADS*5; i++){
     Fifo_Push(&thread_info[i%NUM_THREADS].fifo,i);
     }

 then I appear to get into some sort of deadlock. The call from Fifo_Push never returns. I know I need a critical section in there though.

Any ideas?
0
 
LVL 48

Expert Comment

by:AlexFM
ID: 17993937
About passing thread number: thread doesn't start immediately when CreateThread is called. When thread starts, it gets pointer (in your version) which points to variable i in Initialize_Threads function. At this time i value can change, go out of array range or even go out of scope. This is why I suggested to pass i by value.

About deadlock: there are many places in your functions when function returns without releasing critical section. For example:

CEI_UINT32 Fifo_Pop(FIFO *fifo) {
     
     ELEMENT    *e;
     CEI_UINT32 data;

     EnterCriticalSection(&fifo->critical_section);

     if( fifo->last == fifo->first )
          return fifo->first->data;    // ??? this is only one place, there are others
     
     e = fifo->first;
     fifo->first = fifo->first->next;
     data = e->data;
     free(e);

     LeaveCriticalSection(&fifo->critical_section);

     return data;
}

You need to leave critical section always. In C++ it is possible to write wrapper class which leaves  critical section in destructor. In C you need to leave critical section manually before every return statement.
0
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 17993964
In Fifo_Push there are some returns where you didn't leave the critical section before. That definitively leads to a dead-lock with the next call to EnterCriticalSection.

You easiliy can solve that problem either by adding a LeaveCriticalSection prior to any return or by using that little helper class:

class Locker
{
      CRITICAL_SECTION* m_pcs;
public:
      Locker(CRITICAL_SECTION* pcs) : m_pcs(NULL) { Lock(pcs); }
      ~Locker()  { Unlock();  }
      void Lock(CRITICAL_SECTION* pcs)
       {  if (m_pcs != NULL && m_pcs != pcs)
                Unlock();
           if (m_pcs == NULL)  EnterCriticalSection(pcs);
           m_pcs = pcs;
       }
      void Unlock()
       {  if (m_pcs != NULL) LeaveCriticalSection(m_pcs); m_pcs = NULL; }
};

You can use it like that:

void f()
{
     Locker lock(&GlobalSection);    // the constructor enters

     if  (...)
         return;   // the destructor will leave the critical section

}


Note, critical sections cannot be nested. If you are already locked regarding one critical section you can't lock again beside you would use a second (different) critical section resource.

void f()
{
   EnterCriticalSection(&c1);

    if (...)
    {
         EnterCriticalSection(&c2);     // using c1 would lead to a dead-lock

Same happens by calling another function that locks using a shared critical section object.

class Locker is a little bit more robust but it cannot really solve the problem that critical sections cannot be nested.

Regards, Alex (itsmeandnobodyelse)

0
 

Author Comment

by:DizZzM
ID: 17996708
Amazing the little bugs you over look when coding all day long.... Thanks guys.

Sorry itsmeandnobodyelse but AlexFM really answered this question - the main issue I was having and probably wouldn't of caught solo for a while was the passing by reference to Create_Thread.

Do have one last question though if you guys don't mind:

Kept getting access violation errors once I fixed the FIFO. Looking at my main program, noticed the only issue could be

Thread_Info_Type thread_info[NUM_THREADS];

is a shared resource. However, looking at the code the only shared portion that is currently being (manipulated) used is the Fifo which has its own critical section embedded within.  I wrapped all access to thread_info [] around my global critical section and it fixed the problem. Still not 100% on why it fixed it though - can someone briefly explain? I was under the impression that each item within the array would get it's own memory space and hence be considered it's own single resource. Am I wrong? Should the entire struct - and furthermore entire array of structs be considered a 'single resrouce'?

Additionally - if I do need to wrap every access to the array in a critical section, there's probably no need for me to have a critical section embedded within my fifo right? (Not that it hurts anything if it's synched properly)

Thanks again guys!
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 17997423
>>>> Sorry itsmeandnobodyelse but AlexFM really answered this question

No problem, you decide who gets the points (though it is possible to split points if more than one answer helped).

But you need to insert LeaveCriticalSection prior to each return ... or its definitively a dead-lock.

Regards, Alex
0
 
LVL 48

Expert Comment

by:AlexFM
ID: 17998206
If you access some array (list, etc.) from different threads, with add/remove/edit operations, you need one critical section for all array.
If you create array (list) before starting threads, and then only edit array members in threads, without add/delete operations, you can keep critical section in every array member.
This is general rule. About your access violation problem - I didn't understand exactly what happens. As usual, it is necessary to see code and exact exception information.
0
 

Author Comment

by:DizZzM
ID: 17998873
AlexFM - I opened up a new question to be fair to others who might want to help. The points are also grabs for yourself if you can help relieve me of this headache ;)

http://www.experts-exchange.com/Programming/Programming_Languages/C/Q_22070013.html

0

Featured Post

Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Have you thought about creating an iPhone application (app), but didn't even know where to get started? Here's how: ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ Important pre-programming comments: I’ve never tri…
An Outlet in Cocoa is a persistent reference to a GUI control; it connects a property (a variable) to a control.  For example, it is common to create an Outlet for the text field GUI control and change the text that appears in this field via that Ou…
The goal of this video is to provide viewers with basic examples to understand opening and reading files in the C programming language.
The goal of this video is to provide viewers with basic examples to understand and use conditional statements in the C programming language.
Suggested Courses

650 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question