Can you spot the race condition? (3)

[ClayB]

I was reminded of this code trap from my early days of threaded programming by the thread creation time thread pool question. What's wrong with this method of determining the thread id of a thread?

#include 
#include 
#define N 8
   
pthread_t tid;
   
void* PrintMyID (void* dummy)
{  int k; 
   int myID=-1;
   pthread_t myThreadID;
  
   myThreadID=pthread_self();
   for (k=0; k< N; k++)
     if (pthread_equal(myThreadID, tid) myID = k;
    
   printf ("Hello from thread %d
", myID);   
   return NULL;
}
   
int main ()
{
   int j;
   for (j = 0; j < N; j++)      
     pthread_create (&tid, NULL, PrintMyID, NULL);
   for (j = 0; j < N; j++)
     pthread_join (tid, NULL);
}

Message Edited by intel.software.network.support on 12-09-2005 01:55 PM

[netdevil]

I'm not really familiar with pthreads (I use the winapi exclusively), but it does seem like tid[] isn't initialized to anything when the threads are created, which means the loop checking tid would be bad. Of course, I'm not really sure what pthread_join does ;).

Assuming create starts the worker thread and join is some sort of "waitforobject" type call, and pthread_self() is like GetThreadID(), it would seem like the values in tid[] are pretty unpredictable inside the threads.

Am I at least close? :)
-Dale

[Intel_C_Intel]

for (k=0; k< N; k++)
if (pthread_equal(myThreadID, tid) myID = k;


Is totally out-of-sync with

for (j = 0; j < N; j++)
pthread_create (&tid, NULL, PrintMyID, NULL);

pthread_equal could be called before pthread_create has finished?

[ClayB]

My apologies for being Pthread-centric, Dale. This was the original API that I remember seeing this error and I wasn't sure if the same condition is possible within Win32 threads.

lockfree has hit the nail on the head. The pthread_create function returns (in first parameter) a unique thread identifier and stores these in an array. The threads themselves can find out their own identifier (pthread_self) and then search through the list of saved values. However, since there is no guarantee about when the thread id will be stored in the array slot versus when the spawned thread will start looking for it, it is possible that the thread will not find an identifier to match.

One possible fix to this code would be to put a sleep() call in the thread to (hopefully) wait long enough to have the needed values placed in the array. This still has the potential for missing the value if the sleep interval is not long enough. Another fix would be to have each thread wait for a signal that the "master" thread would send after all threads are created. Better than the first, but this could needlessly stall threads and keep them from computations.

Probably the best solution is to send the required id number as a parameter to the thread function (as seen in the previous "Can you spot the race condition" posts).

-- clay

[ClayB]

Yes, Dale, you have decoded the Pthreads sufficiently to understand the problem. Here's a Win32 threading example of the same code.

#include 
#include 
#define N 8   
  
DWORD tid; 
  
DWORD WINAPI PrintMyID (LPVOID dummy)
{  int k;    
   int myID=-1;
   DWORD myThreadID;
  
   myThreadID=GetCurrentThreadId();
   for (k=0; k< N; k++)
     if (myThreadID == tid) myID = k;
   printf ("Hello from thread %d
", myID);
   return 0;
}
  
   int main ()
{  
   HANDLE hThreads;  
   int j;
 
   for (j = 0; j < N; j++)
       hThreads = CreateThread(NULL, 0, PrintMyID, NULL, 0, &tid);
   
   WaitForMultipleObjects(N, hThreads, TRUE, INFINITE);
}

[bronx]

> Probably the best solution is to send the required id
> number as a parameter to the thread function (as seen

sure for this simple example, but if the actual ThreadId is required in the worker thread, the best is probably to create the thread with CREATE_SUSPENDED in the creation flags and to call ResumeThread right after

[ClayB]

But, if the actual ThreadID is required, the call to GetCurrentThreadID will do the trick. Wouldn't it?

Once the thread has started execution, this value must be available in some kernel object associated with the thread. The race condition stems from the fact that it is (may be?) unknown when the global array will have that threadID stored into. We should expect the array to have this value once the CreateThread call has returned. I'm not sure if there is any specification in Win32 or Pthreads of when the parameters are updated. This is probably implementation independent. Anyone have any ideas on either API?

With the assumption that updated values will be available from parameters passed by reference after the call has completed, Eric's idea could be implemented thusly

for (j = 0; j < N; j++) {
       hThreads = CreateThread(NULL, 
                                  0, 
                                  PrintMyID, 
                                  NULL,
                                  CREATE_SUSPENDED,
                                  &tid);
       ResumeThread(hThreads);
}

I toyed with the idea of doing it all in one statement:

ResumeThread(hThreads=CreateThread(...));

but splitting it apart allows us to check to make sure the thread actually did get created before attempting to resume the suspended thread.

[bronx]

> But, if the actual ThreadID is required, the call to
> GetCurrentThreadID will do the trick. Wouldn't it?

yes for sure, you're right. I've remarked it after posting...

Though, IIRC there is a subtle difference for the thread handle, "GetCurrentThread" returns a pseudo handle that can't be shared with sister threads unlike the handle returned by "CreateThread"



> CreateThread call has returned. I'm not sure if
> there is any specification in Win32 or Pthreads of
> when the parameters are updated. This is probably

both the id and the handle are allocated in the thread creation routine, the return values *must* be initialized when returning from this routine, AFAIK the thread handle is returned in the stack with "_beginthreadex" and "CreateThread" and the address of the id is lost after the call (imagine the id as a local variable, its update can't be safely delayed)


>Eric's idea could be implemented thusly

I'm sure it's fairly safe since I use it routinely since ages in many projects without troubles


> but splitting it apart allows us to check to make
> sure the thread actually did get created before
> attempting to resume the suspended thread.

you're right, if each thread need to access all its sisters' handles you can split it in 2 loops :

  for (j = 0; j < N; j++)
    hThreads = CreateThread(NULL,0,PrintMyID,NULL,CREATE_SUSPENDED,&tid);
  for (j = 0; j < N; j++)
    if (hThreads) ResumeThread(hThreads);