a multithread bug

a multithread bug

• sadyc

  Why are you making each thread own its own critical section The purpose of critical sections is to protect a single instance of data or resource (cout in your case) from being simultaneously accessed by multiple threads.  I suggested a solution earlier that involved only one critical section: how did that turn out

  Brian

   

  
  

• Cari Begle

  "Hanging occasionally" implies the possibility you have a race condition that results in deadlock. I'd like to take a look at your program later, and you're welcome to remind me if I forget.
  

• NathanINTJ

  Thanks for your information.

  I have tried your suggestion but the problem still exists.

  
  

• Jorge Tressino Rua

  The way you're allocating the critical section seems suspect. How about if you just had a single global

  CRITICAL_SECTION myCritSec

  
  Use InitializeCriticalSection( &myCritSec ). Similarly, EnterCriticalSection( &myCritSec ), etc.

  (The fact that you see the docs use LPCRITICAL_SECTION doesn't really mean you need to allocate it on the heap: it means a pointer to a CRITICAL_SECTION structure, and you're allocating 5 pointers to a CRITICAL_SECTION, making cOBJ a pointer to a pointer to a CRITICAL_SECTION. Messy. The way I present it here is usually the way it is done.)

  Also stick to one critical section until you decide that two is really needed.

  Brian

  
  

• Vincent Zhao

  No. I want my program to have one and only one critical section which is shared by 4 threads. I followed your suggestion by creating one variable 'CRITICAL SECTION cOBJ' and removing the array of LPCRITICAL_SECTION.

  However, your suggestion still makes the program hangs occasionally in WINDOWS 2000 environment.

  Here is the fragment of the program:

  #include <iostream>

  #include <strstream>

  #include <string>

  #include <fstream>

  #include <WINDOWS.H>

  #include <WINBASE.H>

  using namespace std;

  #define MAX_THREADS 4

  #define BUF_SIZE 255

  CRITICAL_SECTION cOBJ;

  typedef struct _MyData {

  int val1;

  int val2;

  } MYDATA, *PMYDATA;

  DWORD WINAPI ThreadProc( LPVOID lpParam ) {

  PMYDATA pData;

  pData = (PMYDATA) lpParam;

  EnterCriticalSection(&cOBJ);

  int GGG=1989;

  cout<<"I am thread "<<GGG<<endl;

  LeaveCriticalSection(&cOBJ);

  return 0;

  }

  int main(int argc, char** argv) {

  SYSTEM_INFO sSysInfo;

  PMYDATA pData;

  DWORD dwThreadId[MAX_THREADS];

  HANDLE hThread[MAX_THREADS];

  int i=19;

  for (i=0; i<MAX_THREADS; i++) {

  pData = new MYDATA;

  if (pData == NULL)

  ExitProcess(2);

  pData->val1 = i;

  pData->val2 = i+100;

  InitializeCriticalSection(&cOBJ);

  hThread = CreateThread (

  NULL, 0,ThreadProc,pData,0,&dwThreadId);

  if (hThread == NULL)

  ExitProcess(i);

  }

  WaitForMultipleObjects(MAX_THREADS, hThread, TRUE, INFINITE);

  for (i=0;i<MAX_THREADS; i++)

  CloseHandle(hThread);

  DeleteCriticalSection(&cOBJ);

  return 0;

  }

  
  

• Sanjay Patel

  Correct.

  Initialize the critical section only once.

  Kuphryn

  
  

• cambomj

  InitializeCriticalSection(&cOBJ[threadID]);

  EnterCriticalSection(&cOBJ[threadID]);

  LeaveCriticalSection(&cOBJ[threadID]);

  Kuphryn

  
  

• mbaciak

  First, you're not indexing your array of thread handles and id's in your main. hThread is NOT a handle to a thread. It is an array of threads, and to the compilers eyes, a pointer to a handle to a thread.

  hThread[ i ] = CreateThread ( NULL, 0,ThreadProc,pData,0,&dwThreadId[ i ] );
  if (hThread[ i ] == NULL)
  CloseHandle( hThread[ i ] );

  Didn't your compile generate an error on the first line  

  Note that because HANDLE is typedef'd as void*, you aren't getting the same protection under C++ type-checking.  void*, void**, and void**** all can be passed into a function that takes void* (HANDLE). So you have to be careful here.

  You started out trying to associate a critical section to each thread.  That is wrong: you associate a critical section to a resource (or a sequence of resources) that can only be accessed by a single thread at one time.

  Move your InitializeCriticalSection(&cOBJ); outside the loop to the top of main, and you're in business.

  Brian

   

  
  

• SidC

  Thank you for your suggestion.

  It resolves the problem but each thread uses its own critical section and so the program becomes non-mutually exclusive type.

  
  
• a multithread bug