CODEWITHSUNDEEP
c++multithreadingdata-structures
Gambit King
Gambit King

Reputation: 483

Passing structure to thread using _beginthreadx

#include <iostream>
#include <iomanip>
#include <stdio.h>
#include <windows.h>
#include <process.h>

using namespace std;
HANDLE ghEvents;


struct DataStructure
{
int r[2];
int h;
};


unsigned __stdcall f2(void *p)
{

DataStructure *input = (DataStructure *)p;
int i = (int)input->h;
cout <<i<<endl;

}   


int main()
{

HANDLE hThread[8];
DWORD i, dwEvent, dwThreadID;
unsigned threadID;
DataStructure input;

ghEvents = CreateEvent(NULL,FALSE,FALSE,NULL);  

for (int i = 0; i < 8; i++) 
{   
input.h=i;
hThread[i] = (HANDLE)_beginthreadex( NULL, 0, f2, (void*)&input, 0, &threadID );
}

dwEvent = WaitForMultipleObjects(8,hThread,TRUE,INFINITE);       

CloseHandle(ghEvents); 
for (int i = 0; i < 8; i++) 
{
CloseHandle( hThread[i] );
}

cin.get();
return 0;

}

The output is 77777777 instead of 12345678.

I know i have to pass the input by value and not reference but it keeps giving me an error message, what is the proper way to do it?

Upvotes: 0

Views: 496

Answers (4)

Ed Heal
Ed Heal

Reputation: 60017

This is subsequent to my previous answer as is a better solution if the number of threads is known at compile time.

DataStructure input[8];  

... 

for (int i = 0; i < 8; i++)   
{     
   input[i].h=i;  
   hThread[i] = (HANDLE)_beginthreadex( NULL, 0, f2, (void*)&input[i], 0, &threadID );  
}

And you need to return a value:

unsigned __stdcall f2(void *p)      
{      

   DataStructure *input = (DataStructure *)p;      
   int i = input->h;      
   cout <<i<<endl;      
   return 0; 
}

Upvotes: 1

Ed Heal
Ed Heal

Reputation: 60017

You need to create the data structure for each of the threads as you are overwriting the value of input.h for each of the threads.

So to fix, change it to

DataStructure *input; 

...

for (int i = 0; i < 8; i++)  
{    
   input = new DataStructure ;
   input->h=i; 
   hThread[i] = (HANDLE)_beginthreadex( NULL, 0, f2, (void*)input, 0, &threadID ); 
}

And to avoid a memory leak get the f2 function to delete the input i.e.

unsigned __stdcall f2(void *p)     
{     

   DataStructure *input = (DataStructure *)p;     
   int i = input->h;     
   cout <<i<<endl;     
   delete input;
   return 0;
}

note This solution uses dynamic memory allocation as is good solution if the number of threads are unknown at compile time. See my other answer if the number of threads is known.

Upvotes: 0

Sion Sheevok
Sion Sheevok

Reputation: 4217

You are giving each thread the address of the same DataStructure. Your output is non-deterministic. Depending on when each thread gets to run, they may read before, during, or after another iteration of that loop. Meaning, by the time the thread spins up and gets to access input->h, the main thread may have already continued on and changing input.h to the next iteration.

Example:

  • Main - Loop iteration 0 sets input.h to 0.
  • Main - Loop iteration 0 starts Thread 0 with input as a parameter.
  • Main - Loop iteration 1 sets input.h to 1.
  • Main - Loop iteration 1 starts Thread 1 with input as a parameter.
  • Main - Loop iteration 2 sets input.h to 2.
  • Main - Loop iteration 2 starts Thread 2 with input as a parameter.
  • Thread 1 - Starts up.
  • Thread 0 - Starts up.
  • Main - Loop iteration 3 sets input.h to 3.
  • Thread 0 - Reads input->h as 3.
  • Thread 2 - Starts up.
  • Main - Loop iteration 3 starts Thread 3 with input as a parameter.
  • Thread 1 - Reads input->h as 3.
  • Thread 3 - Starts up.
  • Main - Loop iteration 4 sets input.h to 4.
  • Thread 3 - Reads input->h as 4.
  • Thread 2 - Reads input->h as 4.
  • Thread 4 - Starts up.
  • Thread 4 - Reads input->h as 4.

Final Output: 3344

Give each thread a different DataStructure so that they are not trying to read from the same memory address. This way, there is no race-condition. The term refers to the fact that thread start up and order of execution is not guaranteed, so if threads are accessing the same resources without synchronization being done, they will "race".

Upvotes: 1

Tony The Lion
Tony The Lion

Reputation: 63250

First point is:

You have no synchronization in place for your datastructure which is being passed to multiple threads, and while these threads are doing something with it, you're already going through the next iteration of the loop and changing the value of your datastructure.

Create a new datastructure inside a the loop to avoid having synchronization issues.

Upvotes: 0

Related Questions